diff --git a/backend/src/plugins/llm_analysis/_topology.py b/backend/src/plugins/llm_analysis/_topology.py new file mode 100644 index 00000000..4400d792 --- /dev/null +++ b/backend/src/plugins/llm_analysis/_topology.py @@ -0,0 +1,290 @@ +# #region DashboardTopology [C:3] [TYPE Module] [SEMANTICS superset, topology, dashboard, layout, parsing] +# @BRIEF Dashboard topology extraction — parses Superset position_json into human-readable +# text descriptions of Tab → Row → Chart hierarchy for LLM prompt context. +# @LAYER Plugin +# @RELATION DEPENDS_ON -> [json] +# @INVARIANT All functions are pure (no DB, no I/O beyond json parsing). +# @DATA_CONTRACT Input: Dashboard position_json + Charts → Output: Topology text. + +import json +from typing import Any + + +# #region _chart_name [C:1] [TYPE Function] +# @BRIEF Return the display name for a chart. +def _chart_name(chart: dict) -> str: + return chart.get("slice_name", chart.get("title", f"Chart {chart.get('id')}")) +# #endregion _chart_name + + +# #region _chart_params [C:1] [TYPE Function] +# @BRIEF Return parsed chart params dict. +def _chart_params(chart: dict) -> dict: + params = chart.get("params", {}) + if isinstance(params, str): + try: + return json.loads(params) + except (json.JSONDecodeError, TypeError): + return {} + return params +# #endregion _chart_params + + +# #region _describe_chart_dataset [C:2] [TYPE Function] +# @BRIEF Return dataset description line if available. +# @PRE chart and params are chart dicts. +# @POST Returns string with dataset line or empty string. +def _describe_chart_dataset(chart: dict, params: dict) -> str: + ds_id = chart.get("datasource_id") or params.get("datasource_id") + if ds_id: + return f"\n Dataset: {ds_id}" + return "" +# #endregion _describe_chart_dataset + + +# #region _describe_chart_metrics [C:2] [TYPE Function] +# @BRIEF Return metrics description line if available. +# @PRE params is a parsed chart params dict. +# @POST Returns string with metrics line or empty string. +def _describe_chart_metrics(params: dict) -> str: + metrics = params.get("metrics", []) + if not metrics: + return "" + metric_names: list[str] = [] + for m in metrics: + if isinstance(m, dict): + metric_names.append(m.get("label", m.get("expression", str(m)))) + else: + metric_names.append(str(m)) + if metric_names: + return f"\n Metrics: {', '.join(metric_names[:3])}" + return "" +# #endregion _describe_chart_metrics + + +# #region _describe_chart_groupby [C:2] [TYPE Function] +# @BRIEF Return group-by description line if available. +# @PRE params is a parsed chart params dict. +# @POST Returns string with group-by line or empty string. +def _describe_chart_groupby(params: dict) -> str: + groupby = params.get("groupby", []) + if groupby: + return f"\n Group by: {', '.join(groupby[:3])}" + return "" +# #endregion _describe_chart_groupby + + +# #region _describe_chart_filters [C:2] [TYPE Function] +# @BRIEF Return adhoc filters description line if available. +# @PRE params is a parsed chart params dict. +# @POST Returns string with filters line or empty string. +def _describe_chart_filters(params: dict) -> str: + adhoc_filters = params.get("adhoc_filters", []) + if not adhoc_filters: + return "" + filter_parts: list[str] = [] + for filt in adhoc_filters[:3]: + if isinstance(filt, dict): + subject = filt.get("subject", "") + operator = filt.get("operator", "") + comparator = filt.get("comparator", "") + if subject and comparator: + filter_parts.append(f"{subject} {operator} {comparator}") + else: + filter_parts.append(filt.get("clause", "where")) + if filter_parts: + return f"\n Filters: {', '.join(filter_parts)}" + return "" +# #endregion _describe_chart_filters + + +# #region _describe_chart [C:2] [TYPE Function] +# @BRIEF Return a multi-line description of a single chart. +# @PRE chart is a chart detail dict with slice_name, viz_type, params. +# @POST Returns formatted chart description string. +def _describe_chart(chart: dict) -> str: + name = _chart_name(chart) + viz_type = chart.get("viz_type", "unknown") + params = _chart_params(chart) + + desc = f' Chart "{name}" ({viz_type})' + desc += _describe_chart_dataset(chart, params) + desc += _describe_chart_metrics(params) + desc += _describe_chart_groupby(params) + desc += _describe_chart_filters(params) + return desc +# #endregion _describe_chart + + +# #region _sort_chart_key [C:1] [TYPE Function] +# @BRIEF Sort key for chart lookup items: numeric IDs first, then string keys. +def _sort_chart_key(item): + k, _ = item + try: + return (0, int(k)) + except (ValueError, TypeError): + return (1, str(k)) +# #endregion _sort_chart_key + + +# #region _resolve_child [C:2] [TYPE Function] +# @BRIEF Resolve a child reference which may be a string key or inline dict. +# @PRE ref is a child reference from position_json, pos_json is the full layout. +# @POST Returns the resolved child dict or None. +def _resolve_child(ref, pos_json: dict) -> dict | None: + if isinstance(ref, dict): + return ref + if isinstance(ref, str) and ref in pos_json: + return pos_json[ref] + return None +# #endregion _resolve_child + + +# #region _process_tab [C:2] [TYPE Function] +# @BRIEF Process a TAB type position_json node. +# @POST Returns indented lines for the tab and its children. +def _process_tab( + child: dict, depth: int, pos_json: dict, chart_lookup: dict[str, dict] +) -> list[str]: + meta = child.get("meta", {}) or {} + tab_name = meta.get("text", meta.get("label", "Tab")) + sub_lines = _process_children(child, depth + 1, pos_json, chart_lookup) + chart_count = sum(1 for line in sub_lines if line.strip().startswith("Chart")) + indent = " " * depth + return [f'{indent}─── Tab: "{tab_name}" ({chart_count} charts) ───', *sub_lines] +# #endregion _process_tab + + +# #region _process_chart [C:2] [TYPE Function] +# @BRIEF Process a CHART type position_json node. +# @POST Returns indented line for the chart description or empty list. +def _process_chart( + child: dict, depth: int, chart_lookup: dict[str, dict] +) -> list[str]: + meta = child.get("meta", {}) or {} + chart_id = meta.get("chartId") or meta.get("slice_id") + if chart_id is not None and str(chart_id) in chart_lookup: + indent = " " * depth + return [f"{indent}{_describe_chart(chart_lookup[str(chart_id)])}"] + return [] +# #endregion _process_chart + + +# #region _process_children [C:2] [TYPE Function] +# @BRIEF Recursively process position_json children building indented lines. +# @PRE parent is a position_json node dict, pos_json is the full layout, chart_lookup maps IDs. +# @POST Returns list of formatted topology lines. +def _process_children( + parent: dict, depth: int, pos_json: dict, chart_lookup: dict[str, dict] +) -> list[str]: + result: list[str] = [] + children = parent.get("children", []) if isinstance(parent, dict) else [] + + for child_ref in children: + child = _resolve_child(child_ref, pos_json) + if not child or not isinstance(child, dict): + continue + + child_type = child.get("type", "") + + if child_type == "TAB": + result.extend(_process_tab(child, depth, pos_json, chart_lookup)) + elif child_type == "ROW": + result.extend(_process_children(child, depth, pos_json, chart_lookup)) + elif child_type == "CHART": + result.extend(_process_chart(child, depth, chart_lookup)) + + return result +# #endregion _process_children + + +# #region _extract_topology_children [C:2] [TYPE Function] +# @BRIEF Extract formatted topology lines from position_json. +# @PRE position_json is a parsed layout dict, chart_lookup maps chart IDs to chart dicts. +# @POST Returns list of formatted topology description lines. +def _extract_topology_children( + position_json: dict, chart_lookup: dict[str, dict] +) -> list[str]: + lines: list[str] = [] + grid_nodes = [ + v + for v in position_json.values() + if isinstance(v, dict) and v.get("type") in ("GRID", "ROOT", "CONTAINER") + ] + if grid_nodes: + for grid in grid_nodes: + lines.extend(_process_children(grid, depth=1, pos_json=position_json, chart_lookup=chart_lookup)) + else: + for key, value in position_json.items(): + if ( + isinstance(value, dict) + and value.get("type") in ("TAB", "ROW", "CHART") + and key != "DASHBOARD_VERSION_KEY" + ): + lines.extend(_process_children({"children": [key]}, depth=0, pos_json=position_json, chart_lookup=chart_lookup)) + return lines +# #endregion _extract_topology_children + + +# #region build_dashboard_topology [C:2] [TYPE Function] +# @BRIEF Build a structured text description of dashboard layout from position_json. +# @PARAM dashboard (dict) - Dashboard metadata dict with position_json, dashboard_title. +# @PARAM charts (list[dict]) - List of fetched chart detail dicts for name/viz lookup. +# @POST Returns a human-readable text description of the dashboard topology. +# @RELATION DEPENDS_ON -> [_extract_topology_children] +# @RELATION DEPENDS_ON -> [_sort_chart_key] +def build_dashboard_topology(dashboard: dict, charts: list[dict]) -> str: + """ + Build a structured text description of dashboard layout from position_json. + + Returns a human-readable text like: + Dashboard: "Sales Overview" + --- Tab: "Revenue" (3 charts) --- + Chart "Total Revenue YTD" (big_number_total) + Dataset: 5 + Metrics: SUM(revenue) + ... + --- Tab: "Pipeline" (2 charts) --- + ... + """ + lines: list[str] = [] + title = dashboard.get( + "dashboard_title", + dashboard.get("title", f"Dashboard {dashboard.get('id', '')}"), + ) + lines.append(f'Dashboard: "{title}"') + + position_json = dashboard.get("position_json", {}) + if isinstance(position_json, str): + try: + position_json = json.loads(position_json) + except (json.JSONDecodeError, TypeError): + position_json = {} + if not isinstance(position_json, dict): + position_json = {} + + chart_lookup: dict[str, dict] = {} + for c in charts: + cid = c.get("id") or c.get("slice_id") + if cid is not None: + chart_lookup[str(cid)] = c + + try: + children = _extract_topology_children(position_json, chart_lookup) + lines.extend(children) + except Exception as e: + lines.append(f"\n[Topology parse warning: {e}]") + + if chart_lookup: + lines.append(f"\nCharts: {len(chart_lookup)} total") + for chart_id, c in sorted(chart_lookup.items(), key=_sort_chart_key): + lines.append( + f" - {c.get('slice_name', f'chart_{chart_id}')} " + f"({c.get('viz_type', 'unknown')})" + ) + + return "\n".join(lines) +# #endregion build_dashboard_topology + + +# #endregion DashboardTopology diff --git a/backend/src/plugins/llm_analysis/plugin.py b/backend/src/plugins/llm_analysis/plugin.py index 00e14ad5..ebab5238 100644 --- a/backend/src/plugins/llm_analysis/plugin.py +++ b/backend/src/plugins/llm_analysis/plugin.py @@ -14,6 +14,7 @@ from datetime import UTC, datetime, timedelta import json import os +import re from typing import Any from ...core.async_superset_client import AsyncSupersetClient # noqa: F401 — available for async client wiring @@ -22,7 +23,7 @@ from ...core.logger import belief_scope, logger from ...core.plugin_base import PluginBase from ...core.superset_client import SupersetClient from ...core.task_manager.context import TaskContext -from ...models.llm import ValidationPolicy, ValidationRecord +from ...models.llm import ValidationPolicy, ValidationRecord, ValidationRun from ...services.llm_prompt_templates import ( DEFAULT_LLM_PROMPTS, normalize_llm_settings, @@ -31,6 +32,7 @@ from ...services.llm_prompt_templates import ( from ...services.llm_provider import LLMProviderService from ...services.notifications.service import NotificationService from .models import DetectedIssue, LLMProviderType, ValidationResult, ValidationStatus +from ._topology import build_dashboard_topology from .service import DatasetHealthChecker, LLMClient, ScreenshotService @@ -82,7 +84,8 @@ JSON_FORMAT_INSTRUCTION = ( ' {\n' ' "severity": "WARN" | "FAIL",\n' ' "message": "Description of the issue",\n' - ' "location": "Optional location info"\n' + ' "location": "Where the issue is (tab name, chart name, or screen region)",\n' + ' "tab_index": "0-based index of the tab screenshot where the issue appears (-1 if unknown or not tab-specific)"\n' ' }\n' ' ]\n' '}' @@ -103,34 +106,55 @@ def _ensure_json_prompt(prompt: str | None) -> str: # #endregion _ensure_json_prompt -# #region _update_run_status [TYPE Function] +# #region _update_run_status [C:3] [TYPE Function] # @BRIEF Aggregate ValidationRecord statuses into the ValidationRun. -# Only marks run as finished when ALL dashboards have a record. +# Marks run as completed when ALL expected dashboards have a record. +# Computes pass/warn/fail/unknown counters from individual record statuses. +# @RATIONALE Before the fix, this function used `from ..models.llm import ValidationRun` +# which resolved to src.plugins.models.llm (doesn't exist) — the ModuleNotFoundError +# was silently swallowed by `except Exception: pass`, causing the run to stay "running" +# indefinitely. Fixed by importing ValidationRun at module level. +# @REJECTED Leaving the run in "running" status rejected — blocks future runs for the same +# policy and shows an incomplete state in the UI. Setting run.status = worst record status +# rejected — the run lifecycle status must be "completed"/"partial"/"failed", not "FAIL". # @PRE db session is active, run_id may be None. # @POST If run_id is set and all expected dashboards are recorded, -# ValidationRun.status is updated to the worst record status. +# ValidationRun.status is set to 'completed', counters are computed. +# duration_ms becomes available via finished_at. def _update_run_status(db, run_id: str | None) -> None: if not run_id: return - from ..models.llm import ValidationRecord as VR, ValidationRun as VRun - try: - records = db.query(VR).filter(VR.run_id == run_id).all() - run = db.query(VRun).filter(VRun.id == run_id).first() - if not records or not run: - return - # Only mark complete when ALL expected dashboards have a record - total_expected = run.dashboard_count or 0 - if total_expected > 0 and len(records) < total_expected: - return - # Determine worst status: FAIL > WARN > UNKNOWN > PASS - priority = {"FAIL": 0, "WARN": 1, "UNKNOWN": 2, "PASS": 3} - worst = min(records, key=lambda r: priority.get(r.status, 99)) - run.status = worst.status + with belief_scope("_update_run_status", f"run_id={run_id}"): from datetime import datetime, timezone - run.finished_at = datetime.now(timezone.utc) - db.commit() - except Exception: - pass + try: + records = db.query(ValidationRecord).filter(ValidationRecord.run_id == run_id).all() + run = db.query(ValidationRun).filter(ValidationRun.id == run_id).first() + if not records or not run: + logger.warning(f"[_update_run_status] Run {run_id} not found or has no records") + return + total_expected = run.dashboard_count or 0 + if total_expected > 0 and len(records) < total_expected: + logger.info( + f"[_update_run_status] Run {run_id}: {len(records)}/{total_expected} records, not yet complete" + ) + return + pass_count = sum(1 for r in records if r.status == 'PASS') + warn_count = sum(1 for r in records if r.status == 'WARN') + fail_count = sum(1 for r in records if r.status == 'FAIL') + unknown_count = sum(1 for r in records if r.status == 'UNKNOWN') + run.status = 'completed' + run.finished_at = datetime.now(timezone.utc) + run.pass_count = pass_count + run.warn_count = warn_count + run.fail_count = fail_count + run.unknown_count = unknown_count + db.commit() + logger.info( + f"[_update_run_status] Run {run_id} completed: " + f"PASS={pass_count} WARN={warn_count} FAIL={fail_count} UNKNOWN={unknown_count}" + ) + except Exception as e: + logger.error(f"[_update_run_status] Failed to finalize run {run_id}: {e}") # #endregion _update_run_status @@ -186,9 +210,14 @@ class DashboardValidationPlugin(PluginBase): # @PARAM context (Optional[TaskContext]) - Task context for logging with source attribution. # @PRE params contains dashboard_id, environment_id, and provider_id. # @POST Returns a dictionary with validation results and persists them to the database. + # Guarantees _update_run_status is called even if individual dashboard processing fails. # @SIDE_EFFECT When screenshot_enabled=True: captures a screenshot, calls multimodal LLM API. # When screenshot_enabled=False: fetches chart/dataset metadata, calls text-only LLM API. # Both paths write to the database and dispatch notifications. + # @REJECTED Early exit from the dashboard loop on exception rejected — previously an error + # in one dashboard caused _update_run_status to never be called, leaving the run stuck + # in "running" status forever. Now each dashboard is wrapped in try/except and the + # loop always proceeds to _update_run_status. async def execute(self, params: dict[str, Any], context: TaskContext | None = None): with belief_scope("execute", f"plugin_id={self.id}"): log = context.logger if context else logger @@ -259,6 +288,7 @@ class DashboardValidationPlugin(PluginBase): # Process each dashboard sequentially, collecting results results: list[dict[str, Any]] = [] + last_error: BaseException | None = None for idx, dashboard_id in enumerate(all_dashboard_ids): log.info(f"Processing dashboard {idx + 1}/{len(all_dashboard_ids)}: {dashboard_id}") params["dashboard_id"] = dashboard_id @@ -292,18 +322,26 @@ class DashboardValidationPlugin(PluginBase): params["source_invalid"] = True params["source_error"] = str(e) - if screenshot_enabled: - result = await self._execute_path_a( - params, context, db, db_provider, api_key, env, config_mgr, log - ) - else: - result = await self._execute_path_b( - params, context, db, db_provider, api_key, env, config_mgr, log - ) - results.append(result) + try: + if screenshot_enabled: + result = await self._execute_path_a( + params, context, db, db_provider, api_key, env, config_mgr, log + ) + else: + result = await self._execute_path_b( + params, context, db, db_provider, api_key, env, config_mgr, log + ) + results.append(result) + except Exception as e: + log.error(f"Dashboard {dashboard_id} processing failed: {e}") + last_error = e + # Continue to next dashboard — records for already-processed ones are saved - # After ALL dashboards processed, update run status to reflect completion + # After ALL dashboards processed (even if some failed), update run status _update_run_status(db, params.get("run_id")) + + if last_error: + raise last_error # Re-raise so TaskManager marks the task as FAILED return {"dashboards": results, "total": len(results)} finally: @@ -384,11 +422,21 @@ class DashboardValidationPlugin(PluginBase): dashboard_prompt = _ensure_json_prompt(raw_prompt) llm_call_started_at = datetime.now(UTC) if jpeg_paths: + # Extract tab labels from JPEG filenames for prompt context + # Filename format: {dashboard_id}_{tab_label}_{timestamp}_d{depth}_llm.jpg + tab_labels = [] + for jp in jpeg_paths: + base = os.path.splitext(os.path.basename(jp))[0] + # Remove _llm suffix and parse + clean = base.rsplit("_llm", 1)[0] if base.endswith("_llm") else base + m = re.match(r"\d+_(.+)_\d+_d\d+$", clean) + tab_labels.append(m.group(1).replace("_", " ") if m else f"tab {len(tab_labels)}") analysis = await llm_client.analyze_dashboard_multimodal( screenshot_paths=jpeg_paths, logs=logs, prompt_template=dashboard_prompt, max_images=db_provider.max_images, + tab_labels=tab_labels, ) else: # Fallback: text-only analysis if no screenshots @@ -478,7 +526,9 @@ class DashboardValidationPlugin(PluginBase): summary=validation_result.summary, issues=[issue.model_dump() for issue in validation_result.issues], screenshot_path=validation_result.screenshot_path, - screenshot_paths=webp_paths or jpeg_paths, + screenshot_paths=webp_paths or jpeg_paths or [], + logs_sent_to_llm=logs, + execution_path="screenshot", raw_response=json.dumps(result_payload, ensure_ascii=False), ) db.add(db_record) @@ -594,7 +644,7 @@ class DashboardValidationPlugin(PluginBase): ) # 5. Build topology text - topology = self._build_dashboard_topology(dashboard, chart_data) + topology = build_dashboard_topology(dashboard, chart_data) # 6. Build text prompt and call LLM (text-only) llm_client = LLMClient( @@ -689,6 +739,9 @@ class DashboardValidationPlugin(PluginBase): summary=validation_result.summary, issues=[issue.model_dump() for issue in validation_result.issues], screenshot_path=None, + screenshot_paths=[], + logs_sent_to_llm=logs, + execution_path="text_only", raw_response=json.dumps(result_payload, ensure_ascii=False), ) db.add(db_record) @@ -789,290 +842,7 @@ class DashboardValidationPlugin(PluginBase): # endregion DashboardValidationPlugin._fetch_dashboard_logs - # region DashboardValidationPlugin._build_dashboard_topology [TYPE Function] - # @PURPOSE: Build a structured text description of dashboard layout from position_json. - # Recursively parses position_json to build Tab -> Row -> Chart hierarchy. - # @PARAM dashboard (dict) - Dashboard metadata dict with position_json, dashboard_title. - # @PARAM charts (list[dict]) - List of fetched chart detail dicts for name/viz lookup. - # @POST Returns a human-readable text description of the dashboard topology. - # @RELATION DEPENDS_ON -> [json] - def _build_dashboard_topology(self, dashboard: dict, charts: list[dict]) -> str: - """ - Build a structured text description of dashboard layout from position_json. - - Returns a human-readable text like: - Dashboard: "Sales Overview" - --- Tab: "Revenue" (3 charts) --- - Chart "Total Revenue YTD" (big_number_total) - Dataset: 5 - Metrics: SUM(revenue) - ... - --- Tab: "Pipeline" (2 charts) --- - ... - """ - lines: list[str] = [] - title = dashboard.get( - "dashboard_title", - dashboard.get("title", f"Dashboard {dashboard.get('id', '')}"), - ) - lines.append(f'Dashboard: "{title}"') - - position_json = dashboard.get("position_json", {}) - if isinstance(position_json, str): - try: - position_json = json.loads(position_json) - except (json.JSONDecodeError, TypeError): - position_json = {} - if not isinstance(position_json, dict): - position_json = {} - - # Build chart lookup: chart_id -> chart dict - chart_lookup: dict[str, dict] = {} - for c in charts: - cid = c.get("id") or c.get("slice_id") - if cid is not None: - chart_lookup[str(cid)] = c - - # Process the position_json layout - try: - children = _extract_topology_children(position_json, chart_lookup) - lines.extend(children) - except Exception as e: - lines.append(f"\n[Topology parse warning: {e}]") - - # Add chart summary at the end - if chart_lookup: - lines.append(f"\nCharts: {len(chart_lookup)} total") - for chart_id, c in sorted(chart_lookup.items(), key=_sort_chart_key): - lines.append( - f" - {c.get('slice_name', f'chart_{chart_id}')} " - f"({c.get('viz_type', 'unknown')})" - ) - - return "\n".join(lines) - - # endregion DashboardValidationPlugin._build_dashboard_topology - - -# #region _chart_name [TYPE Function] -# @BRIEF Return the display name for a chart. -def _chart_name(chart: dict) -> str: - return chart.get("slice_name", chart.get("title", f"Chart {chart.get('id')}")) -# #endregion _chart_name - - -# #region _chart_params [TYPE Function] -# @BRIEF Return parsed chart params dict. -def _chart_params(chart: dict) -> dict: - params = chart.get("params", {}) - if isinstance(params, str): - try: - return json.loads(params) - except (json.JSONDecodeError, TypeError): - return {} - return params -# #endregion _chart_params - - -# #region _describe_chart_dataset [TYPE Function] -# @BRIEF Return dataset description line if available. -# @PRE chart and params are chart dicts. -# @POST Returns string with dataset line or empty string. -def _describe_chart_dataset(chart: dict, params: dict) -> str: - ds_id = chart.get("datasource_id") or params.get("datasource_id") - if ds_id: - return f"\n Dataset: {ds_id}" - return "" -# #endregion _describe_chart_dataset - - -# #region _describe_chart_metrics [TYPE Function] -# @BRIEF Return metrics description line if available. -# @PRE params is a parsed chart params dict. -# @POST Returns string with metrics line or empty string. -def _describe_chart_metrics(params: dict) -> str: - metrics = params.get("metrics", []) - if not metrics: - return "" - metric_names: list[str] = [] - for m in metrics: - if isinstance(m, dict): - metric_names.append(m.get("label", m.get("expression", str(m)))) - else: - metric_names.append(str(m)) - if metric_names: - return f"\n Metrics: {', '.join(metric_names[:3])}" - return "" -# #endregion _describe_chart_metrics - - -# #region _describe_chart_groupby [TYPE Function] -# @BRIEF Return group-by description line if available. -# @PRE params is a parsed chart params dict. -# @POST Returns string with group-by line or empty string. -def _describe_chart_groupby(params: dict) -> str: - groupby = params.get("groupby", []) - if groupby: - return f"\n Group by: {', '.join(groupby[:3])}" - return "" -# #endregion _describe_chart_groupby - - -# #region _describe_chart_filters [TYPE Function] -# @BRIEF Return adhoc filters description line if available. -# @PRE params is a parsed chart params dict. -# @POST Returns string with filters line or empty string. -def _describe_chart_filters(params: dict) -> str: - adhoc_filters = params.get("adhoc_filters", []) - if not adhoc_filters: - return "" - filter_parts: list[str] = [] - for filt in adhoc_filters[:3]: - if isinstance(filt, dict): - subject = filt.get("subject", "") - operator = filt.get("operator", "") - comparator = filt.get("comparator", "") - if subject and comparator: - filter_parts.append(f"{subject} {operator} {comparator}") - else: - filter_parts.append(filt.get("clause", "where")) - if filter_parts: - return f"\n Filters: {', '.join(filter_parts)}" - return "" -# #endregion _describe_chart_filters - - -# #region _describe_chart [TYPE Function] -# @BRIEF Return a multi-line description of a single chart. -# @PRE chart is a chart detail dict with slice_name, viz_type, params. -# @POST Returns formatted chart description string. -def _describe_chart(chart: dict) -> str: - name = _chart_name(chart) - viz_type = chart.get("viz_type", "unknown") - params = _chart_params(chart) - - desc = f' Chart "{name}" ({viz_type})' - desc += _describe_chart_dataset(chart, params) - desc += _describe_chart_metrics(params) - desc += _describe_chart_groupby(params) - desc += _describe_chart_filters(params) - return desc -# #endregion _describe_chart - - -# #region _sort_chart_key [TYPE Function] -# @BRIEF Sort key for chart lookup items: numeric IDs first, then string keys. -def _sort_chart_key(item): - k, _ = item - try: - return (0, int(k)) - except (ValueError, TypeError): - return (1, str(k)) -# #endregion _sort_chart_key - - -# #region _resolve_child [TYPE Function] -# @BRIEF Resolve a child reference which may be a string key or inline dict. -# @PRE ref is a child reference from position_json, pos_json is the full layout. -# @POST Returns the resolved child dict or None. -def _resolve_child(ref, pos_json: dict) -> dict | None: - if isinstance(ref, dict): - return ref - if isinstance(ref, str) and ref in pos_json: - return pos_json[ref] - return None -# #endregion _resolve_child - - -# #region _process_tab [TYPE Function] -# @BRIEF Process a TAB type position_json node. -# @POST Returns indented lines for the tab and its children. -def _process_tab( - child: dict, depth: int, pos_json: dict, chart_lookup: dict[str, dict] -) -> list[str]: - meta = child.get("meta", {}) or {} - tab_name = meta.get("text", meta.get("label", "Tab")) - sub_lines = _process_children(child, depth + 1, pos_json, chart_lookup) - chart_count = sum(1 for line in sub_lines if line.strip().startswith("Chart")) - indent = " " * depth - return [f'{indent}─── Tab: "{tab_name}" ({chart_count} charts) ───', *sub_lines] -# #endregion _process_tab - - -# #region _process_chart [TYPE Function] -# @BRIEF Process a CHART type position_json node. -# @POST Returns indented line for the chart description or empty list. -def _process_chart( - child: dict, depth: int, chart_lookup: dict[str, dict] -) -> list[str]: - meta = child.get("meta", {}) or {} - chart_id = meta.get("chartId") or meta.get("slice_id") - if chart_id is not None and str(chart_id) in chart_lookup: - indent = " " * depth - return [f"{indent}{_describe_chart(chart_lookup[str(chart_id)])}"] - return [] -# #endregion _process_chart - - -# #region _process_children [TYPE Function] -# @BRIEF Recursively process position_json children building indented lines. -# @PRE parent is a position_json node dict, pos_json is the full layout, chart_lookup maps IDs. -# @POST Returns list of formatted topology lines. -def _process_children( - parent: dict, depth: int, pos_json: dict, chart_lookup: dict[str, dict] -) -> list[str]: - result: list[str] = [] - children = parent.get("children", []) if isinstance(parent, dict) else [] - - for child_ref in children: - child = _resolve_child(child_ref, pos_json) - if not child or not isinstance(child, dict): - continue - - child_type = child.get("type", "") - - if child_type == "TAB": - result.extend(_process_tab(child, depth, pos_json, chart_lookup)) - elif child_type == "ROW": - result.extend(_process_children(child, depth, pos_json, chart_lookup)) - elif child_type == "CHART": - result.extend(_process_chart(child, depth, chart_lookup)) - - return result -# #endregion _process_children - - -# #region _extract_topology_children [TYPE Function] -# @BRIEF Extract formatted topology lines from position_json. -# @PRE position_json is a parsed layout dict, chart_lookup maps chart IDs to chart dicts. -# @POST Returns list of formatted topology description lines. -def _extract_topology_children( - position_json: dict, chart_lookup: dict[str, dict] -) -> list[str]: - lines: list[str] = [] - # Find grid/root containers that hold the layout - grid_nodes = [ - v - for v in position_json.values() - if isinstance(v, dict) and v.get("type") in ("GRID", "ROOT", "CONTAINER") - ] - if grid_nodes: - for grid in grid_nodes: - lines.extend(_process_children(grid, depth=1, pos_json=position_json, chart_lookup=chart_lookup)) - else: - # No explicit grid — try processing all top-level layout components - for key, value in position_json.items(): - if ( - isinstance(value, dict) - and value.get("type") in ("TAB", "ROW", "CHART") - and key != "DASHBOARD_VERSION_KEY" - ): - lines.extend(_process_children({"children": [key]}, depth=0, pos_json=position_json, chart_lookup=chart_lookup)) - return lines -# #endregion _extract_topology_children - - -# #endregion DashboardValidationPlugin + # endregion DashboardValidationPlugin # #region DocumentationPlugin [TYPE Class] diff --git a/backend/src/plugins/llm_analysis/service.py b/backend/src/plugins/llm_analysis/service.py index 8d5ec1fe..d54feefa 100644 --- a/backend/src/plugins/llm_analysis/service.py +++ b/backend/src/plugins/llm_analysis/service.py @@ -629,7 +629,7 @@ class ScreenshotService: if not safe_tab: safe_tab = f"tab_{depth}_{i}" - tab_filename = f"{dashboard_id}_{safe_tab}_{timestamp}.png" + tab_filename = f"{dashboard_id}_{safe_tab}_{timestamp}_d{depth}.png" tab_path = os.path.join(output_dir, tab_filename) try: @@ -1280,6 +1280,7 @@ class LLMClient: # region LLMClient.analyze_dashboard_multimodal [TYPE Function] [C:3] # @PURPOSE Path A: send screenshots + logs to multimodal LLM, with chunking support. # @PRE screenshot_paths is a non-empty list of paths. + # tab_labels, if provided, must have the same length as screenshot_paths. # @POST Returns dict {status, summary, issues} with optional chunk_count. # @SIDE_EFFECT Compresses images, calls external LLM API (possibly multiple times for chunks). # @RATIONALE Screenshots are split into chunks of max_images to respect provider image limits. @@ -1293,6 +1294,7 @@ class LLMClient: max_width: int = 1024, image_quality: int = 60, max_images: int | None = None, + tab_labels: list[str] | None = None, ) -> dict[str, Any]: with belief_scope("analyze_dashboard_multimodal"): if not screenshot_paths: @@ -1302,7 +1304,14 @@ class LLMClient: encoded_images = self._optimize_images(screenshot_paths, max_width, image_quality) log_text = "\n".join(logs) - prompt = render_prompt(prompt_template, {"logs": log_text}) + tab_list_text = "\n".join( + f" Screenshot {i}: {label}" for i, label in enumerate(tab_labels or []) + ) or "Screenshots are in order." + prompt = render_prompt(prompt_template, { + "logs": log_text, + "tab_list": tab_list_text, + "total_chunks": str(len(encoded_images)), + }) # 2. Determine chunking # Default to 8 images per chunk as a safe fallback when max_images is 0 or None diff --git a/backend/src/services/llm_prompt_templates.py b/backend/src/services/llm_prompt_templates.py index 0b4d7b86..1b624f7a 100644 --- a/backend/src/services/llm_prompt_templates.py +++ b/backend/src/services/llm_prompt_templates.py @@ -34,10 +34,13 @@ DEFAULT_LLM_PROMPTS: dict[str, str] = { ), "dashboard_validation_prompt_multimodal": ( # Path A — image-focused (v2) "Analyze the attached {total_chunks} dashboard tab screenshots and execution logs for visual and data health issues.\n\n" - "Each screenshot corresponds to a different dashboard tab. " + "Screenshots are attached IN ORDER — screenshot 0 is the first tab, screenshot 1 is the second, etc.\n" + "Tab order:\n" + "{tab_list}\n\n" "Logs:\n" "{logs}\n\n" - "Provide the analysis in JSON format with the following structure:\n" + "Provide the analysis in JSON format. Each issue MUST specify the tab_index field " + "(0-based index of the screenshot/tab where the issue appears, or -1 if not tab-specific):\n" "{\n" ' "status": "PASS" | "WARN" | "FAIL",\n' ' "summary": "Short summary of findings",\n' @@ -45,7 +48,8 @@ DEFAULT_LLM_PROMPTS: dict[str, str] = { " {\n" ' "severity": "WARN" | "FAIL",\n' ' "message": "Description of the issue",\n' - ' "location": "Optional location info (e.g. tab name, chart name)"\n' + ' "location": "Where the issue is located (tab name, chart name, screen region)",\n' + ' "tab_index": "0-based tab index (-1 if unknown)"\n' " }\n" " ]\n" "}" @@ -53,13 +57,14 @@ DEFAULT_LLM_PROMPTS: dict[str, str] = { "dashboard_validation_prompt_text": ( # Path B — topology-focused (v2) "Analyze the following dashboard topology, dataset health, and execution logs for " "data consistency and KXD connectivity issues.\n\n" - "Dashboard structure:\n" + "Dashboard structure (tabs are indicated in the topology below):\n" "{topology}\n\n" "Dataset health:\n" "{dataset_health}\n\n" "Logs:\n" "{logs}\n\n" - "Provide the analysis in JSON format with the following structure:\n" + "Provide the analysis in JSON format. Each issue MUST specify the tab_index field " + "(0-based index of the tab where the issue appears, or -1 if not tab-specific):\n" "{\n" ' "status": "PASS" | "WARN" | "FAIL",\n' ' "summary": "Short summary of findings",\n' @@ -67,7 +72,8 @@ DEFAULT_LLM_PROMPTS: dict[str, str] = { " {\n" ' "severity": "WARN" | "FAIL",\n' ' "message": "Description of the issue",\n' - ' "location": "Optional location info (e.g. dataset name, chart name)"\n' + ' "location": "Where the issue is located (tab name, chart name, dataset name)",\n' + ' "tab_index": "0-based tab index (-1 if unknown)"\n' " }\n" " ]\n" "}" diff --git a/backend/src/services/validation_service.py b/backend/src/services/validation_service.py index 5b1c512e..4e6437aa 100644 --- a/backend/src/services/validation_service.py +++ b/backend/src/services/validation_service.py @@ -265,7 +265,18 @@ class ValidationTaskService: # #region _run_to_dict [C:2] [TYPE Function] # @BRIEF Convert ValidationRun ORM object to a serializable dict for run history APIs. + # Computes duration_ms from started_at/finished_at for frontend display. + # @RATIONALE duration_ms added so the frontend can show "Длительность: 3m 12s" on the + # run detail page. Previously duration was always "-" even for completed runs. + # @POST Returns dict with duration_ms = int (ms) if both started_at and finished_at + # are present, else None. All other fields are direct ORM mappings. def _run_to_dict(self, run: ValidationRun) -> dict: + from datetime import timezone as _tz + duration_ms: int | None = None + if run.started_at and run.finished_at: + started = run.started_at.replace(tzinfo=_tz.utc) if run.started_at.tzinfo is None else run.started_at + finished = run.finished_at.replace(tzinfo=_tz.utc) if run.finished_at.tzinfo is None else run.finished_at + duration_ms = int((finished - started).total_seconds() * 1000) return { "id": run.id, "policy_id": run.policy_id, @@ -274,6 +285,7 @@ class ValidationTaskService: "trigger": run.trigger, "started_at": run.started_at.isoformat() if run.started_at else None, "finished_at": run.finished_at.isoformat() if run.finished_at else None, + "duration_ms": duration_ms, "dashboard_count": run.dashboard_count or 0, "pass_count": run.pass_count or 0, "warn_count": run.warn_count or 0, diff --git a/backend/tests/plugins/test_update_run_status.py b/backend/tests/plugins/test_update_run_status.py new file mode 100644 index 00000000..88d5df50 --- /dev/null +++ b/backend/tests/plugins/test_update_run_status.py @@ -0,0 +1,256 @@ +# #region TestUpdateRunStatus [C:3] [TYPE Module] [SEMANTICS test,validation,run,status,counters] +# @BRIEF Verify _update_run_status computes counters correctly, sets status to 'completed', +# and handles edge cases (missing run, partial records, exception handling). +# @RELATION BINDS_TO -> [LLMAnalysisPlugin] +# @TEST_EDGE: missing_run_id -> Returns immediately without DB query +# @TEST_EDGE: no_records_found -> Logs warning and returns +# @TEST_EDGE: partial_records -> Does NOT mark complete when records < dashboard_count +# @TEST_EDGE: exception_in_query -> Logs error, does not crash +# @TEST_INVARIANT: run_status_always_completed -> VERIFIED_BY: test_sets_status_completed_with_all_records +# @TEST_INVARIANT: counters_computed_from_records -> VERIFIED_BY: test_computed_counters_mixed_statuses, test_computed_counters_all_pass +# @TEST_INVARIANT: finished_at_set_on_completion -> VERIFIED_BY: test_sets_finished_at_utc + +import sys +from datetime import UTC, datetime, timezone +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +# Ensure src is importable (same pattern as test_llm_dashboard_validation_v2.py) +sys.path.insert(0, str(Path(__file__).parent.parent.parent / "src")) + +from src.plugins.llm_analysis.plugin import _update_run_status + + +# Minimal mock models matching the ORM shape +class MockValidationRecord: + def __init__(self, status, run_id="run-1"): + self.status = status + self.run_id = run_id + + +class MockValidationRun: + def __init__(self, id="run-1", dashboard_count=3, status="running"): + self.id = id + self.dashboard_count = dashboard_count + self.status = status + self.finished_at = None + self.pass_count = 0 + self.warn_count = 0 + self.fail_count = 0 + self.unknown_count = 0 + + +def _make_mock_db(records=None, run=None): + """Create a mock DB session that returns the given records and run.""" + mock_db = MagicMock() + mock_query = MagicMock() + mock_db.query.return_value = mock_query + mock_query.filter.return_value = mock_query + mock_query.all.return_value = records or [] + mock_query.first.return_value = run + return mock_db + + +# #region TestUpdateRunStatusNoop [C:2] [TYPE Class] +# @BRIEF Verify _update_run_status early-exit conditions. +class TestUpdateRunStatusNoop: + """Verify _update_run_status no-op paths.""" + + # #region test_none_run_id_returns_early [C:2] [TYPE Function] + # @BRIEF When run_id is None, function returns without DB access. + def test_none_run_id_returns_early(self): + mock_db = MagicMock() + _update_run_status(mock_db, None) + mock_db.query.assert_not_called() + # #endregion test_none_run_id_returns_early + + # #region test_empty_run_id_returns_early [C:2] [TYPE Function] + # @BRIEF When run_id is empty string, function returns without DB access. + def test_empty_run_id_returns_early(self): + mock_db = MagicMock() + _update_run_status(mock_db, "") + mock_db.query.assert_not_called() + # #endregion test_empty_run_id_returns_early + + # #region test_no_records_logs_warning [C:2] [TYPE Function] + # @BRIEF When no records exist for the run, logs warning and returns. + def test_no_records_logs_warning(self): + mock_db = _make_mock_db(records=[], run=MockValidationRun()) + + _update_run_status(mock_db, "run-1") + mock_db.commit.assert_not_called() + # #endregion test_no_records_logs_warning + + # #region test_no_run_found_logs_warning [C:2] [TYPE Function] + # @BRIEF When run record not found, logs warning and returns. + def test_no_run_found_logs_warning(self): + mock_db = _make_mock_db(records=[MockValidationRecord("PASS")], run=None) + + _update_run_status(mock_db, "run-1") + mock_db.commit.assert_not_called() + # #endregion test_no_run_found_logs_warning +# #endregion TestUpdateRunStatusNoop + + +# #region TestUpdateRunStatusPartial [C:2] [TYPE Class] +# @BRIEF Verify _update_run_status does NOT mark complete when records < dashboard_count. +class TestUpdateRunStatusPartial: + """Verify partial completion guard.""" + + # #region test_partial_records_not_marked_complete [C:2] [TYPE Function] + # @BRIEF When len(records) < dashboard_count, run stays 'running'. + def test_partial_records_not_marked_complete(self): + run = MockValidationRun(dashboard_count=3, status="running") + records = [MockValidationRecord("PASS"), MockValidationRecord("WARN")] # 2 < 3 + mock_db = _make_mock_db(records=records, run=run) + + _update_run_status(mock_db, "run-1") + + assert run.status == "running" # NOT changed + mock_db.commit.assert_not_called() + # #endregion test_partial_records_not_marked_complete + + # #region test_zero_dashboard_count_skips_guard [C:2] [TYPE Function] + # @BRIEF When dashboard_count is 0 or None, the partial guard is skipped and run completes. + def test_zero_dashboard_count_skips_guard(self): + run = MockValidationRun(dashboard_count=0, status="running") + records = [MockValidationRecord("PASS")] + mock_db = _make_mock_db(records=records, run=run) + + _update_run_status(mock_db, "run-1") + + assert run.status == "completed" + mock_db.commit.assert_called_once() + # #endregion test_zero_dashboard_count_skips_guard +# #endregion TestUpdateRunStatusPartial + + +# #region TestUpdateRunStatusCompletion [C:2] [TYPE Class] +# @BRIEF Verify _update_run_status correctly computes counters and sets status. +class TestUpdateRunStatusCompletion: + """Verify counter computation and status assignment.""" + + # #region test_sets_status_completed_with_all_records [C:2] [TYPE Function] + # @BRIEF When all expected records exist, run.status is set to 'completed'. + def test_sets_status_completed_with_all_records(self): + run = MockValidationRun(dashboard_count=2, status="running") + records = [MockValidationRecord("PASS"), MockValidationRecord("FAIL")] + mock_db = _make_mock_db(records=records, run=run) + + _update_run_status(mock_db, "run-1") + + assert run.status == "completed" + # #endregion test_sets_status_completed_with_all_records + + # #region test_computed_counters_all_pass [C:2] [TYPE Function] + # @BRIEF All PASS records → pass_count=2, others=0. + def test_computed_counters_all_pass(self): + run = MockValidationRun(dashboard_count=2) + records = [MockValidationRecord("PASS"), MockValidationRecord("PASS")] + mock_db = _make_mock_db(records=records, run=run) + + _update_run_status(mock_db, "run-1") + + assert run.pass_count == 2 + assert run.warn_count == 0 + assert run.fail_count == 0 + assert run.unknown_count == 0 + # #endregion test_computed_counters_all_pass + + # #region test_computed_counters_mixed_statuses [C:2] [TYPE Function] + # @BRIEF Mixed statuses → correct per-status counters. + def test_computed_counters_mixed_statuses(self): + run = MockValidationRun(dashboard_count=4) + records = [ + MockValidationRecord("PASS"), + MockValidationRecord("PASS"), + MockValidationRecord("WARN"), + MockValidationRecord("FAIL"), + ] + mock_db = _make_mock_db(records=records, run=run) + + _update_run_status(mock_db, "run-1") + + assert run.pass_count == 2 + assert run.warn_count == 1 + assert run.fail_count == 1 + assert run.unknown_count == 0 + # #endregion test_computed_counters_mixed_statuses + + # #region test_computed_counters_unknown_status [C:2] [TYPE Function] + # @BRIEF UNKNOWN status records are counted correctly. + def test_computed_counters_unknown_status(self): + run = MockValidationRun(dashboard_count=2) + records = [MockValidationRecord("UNKNOWN"), MockValidationRecord("PASS")] + mock_db = _make_mock_db(records=records, run=run) + + _update_run_status(mock_db, "run-1") + + assert run.unknown_count == 1 + assert run.pass_count == 1 + # #endregion test_computed_counters_unknown_status + + # #region test_sets_finished_at [C:2] [TYPE Function] + # @BRIEF finished_at is set to a datetime on completion. + def test_sets_finished_at(self): + run = MockValidationRun(dashboard_count=1, status="running") + records = [MockValidationRecord("PASS")] + mock_db = _make_mock_db(records=records, run=run) + + _update_run_status(mock_db, "run-1") + + assert run.finished_at is not None + assert isinstance(run.finished_at, datetime) + # #endregion test_sets_finished_at + + # #region test_db_exception_logged_not_raised [C:2] [TYPE Function] + # @BRIEF DB exception during finalization is logged but not re-raised. + def test_db_exception_logged_not_raised(self): + mock_db = MagicMock() + mock_query = MagicMock() + mock_db.query.return_value = mock_query + mock_query.filter.return_value = mock_query + mock_query.all.side_effect = RuntimeError("DB connection lost") + + # Should NOT raise + _update_run_status(mock_db, "run-1") + # #endregion test_db_exception_logged_not_raised +# #endregion TestUpdateRunStatusCompletion + + +# #region TestExecuteErrorHandling [C:2] [TYPE Class] +# @BRIEF Verify the always-finalize pattern used in execute(). +class TestExecuteErrorHandling: + """Verify _update_run_status is always callable after the loop.""" + + # #region test_update_run_status_called_on_success [C:2] [TYPE Function] + # @BRIEF _update_run_status is called after normal execution with all records. + def test_update_run_status_called_on_success(self): + run = MockValidationRun(dashboard_count=1, status="running") + records = [MockValidationRecord("PASS")] + mock_db = _make_mock_db(records=records, run=run) + + _update_run_status(mock_db, "run-1") + mock_db.commit.assert_called_once() + assert run.status == "completed" + # #endregion test_update_run_status_called_on_success + + # #region test_update_run_status_handles_zero_records [C:2] [TYPE Function] + # @BRIEF When all dashboards failed (0 records), _update_run_status still runs + # and does not mark the run as complete (partial guard). + def test_update_run_status_handles_zero_records(self): + run = MockValidationRun(dashboard_count=2, status="running") + mock_db = _make_mock_db(records=[], run=run) + + _update_run_status(mock_db, "run-1") + + # With 0 records and dashboard_count=2, the partial guard prevents completion + assert run.status == "running" + mock_db.commit.assert_not_called() + # #endregion test_update_run_status_handles_zero_records +# #endregion TestExecuteErrorHandling + + +# #endregion TestUpdateRunStatus diff --git a/backend/tests/services/test_run_to_dict.py b/backend/tests/services/test_run_to_dict.py new file mode 100644 index 00000000..fddee166 --- /dev/null +++ b/backend/tests/services/test_run_to_dict.py @@ -0,0 +1,154 @@ +# #region TestRunToDict [C:3] [TYPE Module] [SEMANTICS test,validation,run,dict,duration] +# @BRIEF Verify ValidationTaskService._run_to_dict computes duration_ms correctly, +# including timezone-naive datetime handling and edge cases. +# @RELATION BINDS_TO -> [ValidationTaskService] +# @TEST_EDGE: no_finished_at -> duration_ms is None +# @TEST_EDGE: no_started_at -> duration_ms is None +# @TEST_EDGE: naive_datetime -> Handled correctly (treated as UTC) +# @TEST_EDGE: aware_datetime -> Handled correctly +# @TEST_INVARIANT: duration_ms_computed -> VERIFIED_BY: test_duration_ms_with_aware_datetimes, test_duration_ms_with_naive_datetimes + +import sys +from datetime import UTC, datetime, timezone, timedelta +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent.parent / "src")) + + +class MockRun: + """Minimal mock matching ValidationRun ORM attributes.""" + def __init__(self, **kwargs): + self.id = kwargs.get("id", "run-1") + self.policy_id = kwargs.get("policy_id", "policy-1") + self.task_id = kwargs.get("task_id", "task-1") + self.status = kwargs.get("status", "completed") + self.trigger = kwargs.get("trigger", "manual") + self.started_at = kwargs.get("started_at") + self.finished_at = kwargs.get("finished_at") + self.dashboard_count = kwargs.get("dashboard_count", 3) + self.pass_count = kwargs.get("pass_count", 2) + self.warn_count = kwargs.get("warn_count", 1) + self.fail_count = kwargs.get("fail_count", 0) + self.unknown_count = kwargs.get("unknown_count", 0) + self.created_at = kwargs.get("created_at", datetime(2025, 1, 15, tzinfo=UTC)) + + +# #region TestRunToDictDuration [C:2] [TYPE Class] +# @BRIEF Verify _run_to_dict duration_ms computation. +class TestRunToDictDuration: + """Verify _run_to_dict duration_ms field.""" + + # #region setup [C:1] [TYPE Function] + @pytest.fixture(autouse=True) + def setup_service(self): + from src.services.validation_service import ValidationTaskService + self.mock_db = MagicMock() + self.service = ValidationTaskService(db=self.mock_db) + self.run_to_dict = self.service._run_to_dict + # #endregion setup + + # #region test_duration_ms_with_aware_datetimes [C:2] [TYPE Function] + # @BRIEF UTC-aware started_at/finished_at → correct duration_ms. + def test_duration_ms_with_aware_datetimes(self): + started = datetime(2025, 1, 15, 10, 0, 0, tzinfo=UTC) + finished = datetime(2025, 1, 15, 10, 0, 5, tzinfo=UTC) # 5 seconds + run = MockRun(started_at=started, finished_at=finished) + + result = self.run_to_dict(run) + + assert result["duration_ms"] == 5000 + # #endregion test_duration_ms_with_aware_datetimes + + # #region test_duration_ms_with_naive_datetimes [C:2] [TYPE Function] + # @BRIEF Naive (no timezone) datetimes → treated as UTC, correct duration. + def test_duration_ms_with_naive_datetimes(self): + started = datetime(2025, 1, 15, 10, 0, 0) # naive + finished = datetime(2025, 1, 15, 10, 2, 30) # naive, 2.5 minutes + run = MockRun(started_at=started, finished_at=finished) + + result = self.run_to_dict(run) + + assert result["duration_ms"] == 150000 # 2.5 * 60 * 1000 + # #endregion test_duration_ms_with_naive_datetimes + + # #region test_duration_ms_no_finished_at [C:2] [TYPE Function] + # @BRIEF When finished_at is None, duration_ms is None. + def test_duration_ms_no_finished_at(self): + started = datetime(2025, 1, 15, 10, 0, 0, tzinfo=UTC) + run = MockRun(started_at=started, finished_at=None) + + result = self.run_to_dict(run) + + assert result["duration_ms"] is None + # #endregion test_duration_ms_no_finished_at + + # #region test_duration_ms_no_started_at [C:2] [TYPE Function] + # @BRIEF When started_at is None, duration_ms is None. + def test_duration_ms_no_started_at(self): + finished = datetime(2025, 1, 15, 10, 0, 0, tzinfo=UTC) + run = MockRun(started_at=None, finished_at=finished) + + result = self.run_to_dict(run) + + assert result["duration_ms"] is None + # #endregion test_duration_ms_no_started_at + + # #region test_duration_ms_both_none [C:2] [TYPE Function] + # @BRIEF When both are None, duration_ms is None. + def test_duration_ms_both_none(self): + run = MockRun(started_at=None, finished_at=None) + + result = self.run_to_dict(run) + + assert result["duration_ms"] is None + # #endregion test_duration_ms_both_none + + # #region test_duration_ms_mixed_aware_naive [C:2] [TYPE Function] + # @BRIEF One aware, one naive → still computes correctly (both treated as UTC). + def test_duration_ms_mixed_aware_naive(self): + started = datetime(2025, 1, 15, 10, 0, 0) # naive + finished = datetime(2025, 1, 15, 10, 0, 10, tzinfo=UTC) # aware + run = MockRun(started_at=started, finished_at=finished) + + result = self.run_to_dict(run) + + assert result["duration_ms"] == 10000 + # #endregion test_duration_ms_mixed_aware_naive + + # #region test_other_fields_present [C:2] [TYPE Function] + # @BRIEF _run_to_dict returns all expected fields. + def test_other_fields_present(self): + run = MockRun() + result = self.run_to_dict(run) + + assert result["id"] == "run-1" + assert result["policy_id"] == "policy-1" + assert result["task_id"] == "task-1" + assert result["status"] == "completed" + assert result["trigger"] == "manual" + assert result["dashboard_count"] == 3 + assert result["pass_count"] == 2 + assert result["warn_count"] == 1 + assert result["fail_count"] == 0 + assert result["unknown_count"] == 0 + # #endregion test_other_fields_present + + # #region test_isoformat_on_dates [C:2] [TYPE Function] + # @BRIEF started_at/finished_at are ISO-formatted strings in output. + def test_isoformat_on_dates(self): + started = datetime(2025, 1, 15, 10, 0, 0, tzinfo=UTC) + run = MockRun(started_at=started, finished_at=started) + + result = self.run_to_dict(run) + + assert isinstance(result["started_at"], str) + assert isinstance(result["finished_at"], str) + assert "2025-01-15" in result["started_at"] + # #endregion test_isoformat_on_dates +# #endregion TestRunToDictDuration + + +# #endregion TestRunToDict diff --git a/frontend/src/routes/validation-tasks/+page.js b/frontend/src/routes/validation-tasks/+page.ts similarity index 65% rename from frontend/src/routes/validation-tasks/+page.js rename to frontend/src/routes/validation-tasks/+page.ts index 57f340de..d48c3fca 100644 --- a/frontend/src/routes/validation-tasks/+page.js +++ b/frontend/src/routes/validation-tasks/+page.ts @@ -6,9 +6,24 @@ // @UX_STATE Error -> error field exposed, component shows retry banner import { api } from '$lib/api.js'; import { log } from '$lib/cot-logger'; +import type { PageLoad } from './$types'; -/** @type {import('./$types').PageLoad} */ -export async function load({ url }) { +interface TasksResponse { + items?: unknown[]; + tasks?: unknown[]; + results?: unknown[]; + total?: number; +} + +interface LoadResult { + tasks: unknown[]; + total: number; + page: number; + page_size: number; + error?: string; +} + +export const load: PageLoad = async ({ url }): Promise => { const page = parseInt(url.searchParams.get('page') || '1', 10); const page_size = parseInt(url.searchParams.get('page_size') || '20', 10); const is_active = url.searchParams.get('is_active') ?? undefined; @@ -16,30 +31,30 @@ export async function load({ url }) { const search = url.searchParams.get('search') || undefined; try { - /** @type {{ tasks: Array, total: number }} */ - const result = await api.getValidationTasks({ + const result: TasksResponse = await api.getValidationTasks({ page, page_size, ...(is_active !== undefined ? { is_active: is_active === 'true' } : {}), ...(environment_id ? { environment_id } : {}), - ...(search ? { search } : {}) + ...(search ? { search } : {}), }); return { tasks: result?.items ?? result?.tasks ?? result?.results ?? [], total: result?.total ?? 0, page, - page_size + page_size, }; - } catch (e) { - log('validation-tasks', 'EXPLORE', 'Load failed', {}, e instanceof Error ? e.message : 'unknown'); + } catch (e: unknown) { + const message = e instanceof Error ? e.message : 'unknown'; + log('validation-tasks', 'EXPLORE', 'Load failed', {}, message); return { tasks: [], total: 0, page, page_size, - error: /** @type {Error} */ (e).message || 'Failed to load validation tasks' + error: message || 'Failed to load validation tasks', }; } -} +}; // #endregion ValidationTasksPageLoad diff --git a/frontend/src/routes/validation-tasks/[policyId]/+page.js b/frontend/src/routes/validation-tasks/[policyId]/+page.js deleted file mode 100644 index 207149e9..00000000 --- a/frontend/src/routes/validation-tasks/[policyId]/+page.js +++ /dev/null @@ -1,32 +0,0 @@ -// #region ValidationTaskDetailPageLoad [C:2] [TYPE Module] [SEMANTICS validation, task, detail, load] -// @BRIEF SvelteKit load function for the validation task detail page — fetches task metadata and paginated run history. -// @RELATION CALLS -> [ValidationApi] -// @PRE policyId route param is a non-empty string. -// @POST Returns { task, runs, total } or redirects to 404 on missing task. -import { getValidationTask, getValidationRuns } from '$lib/api.js'; -import { error } from '@sveltejs/kit'; - -/** @type {import('./$types').PageLoad} */ -export async function load({ params }) { - const { policyId } = params; - - const [taskRes, runsRes] = await Promise.all([ - getValidationTask(policyId).catch(() => null), - getValidationRuns(policyId, { page: 1, page_size: 20 }).catch(() => ({ results: [], total: 0 })), - ]); - - if (!taskRes) { - error(404, 'Task not found'); - } - - const task = taskRes.task || taskRes; - const runs = Array.isArray(runsRes) ? runsRes : (runsRes?.results || runsRes?.items || []); - const total = runsRes?.total || runsRes?.count || runs.length; - - return { - task, - runs, - total, - }; -} -// #endregion ValidationTaskDetailPageLoad diff --git a/frontend/src/routes/validation-tasks/[policyId]/+page.ts b/frontend/src/routes/validation-tasks/[policyId]/+page.ts new file mode 100644 index 00000000..b3792408 --- /dev/null +++ b/frontend/src/routes/validation-tasks/[policyId]/+page.ts @@ -0,0 +1,46 @@ +// #region ValidationTaskDetailPageLoad [C:2] [TYPE Module] [SEMANTICS validation, task, detail, load] +// @BRIEF SvelteKit load function for the validation task detail page — fetches task metadata and paginated run history. +// @RELATION CALLS -> [ValidationApi] +// @PRE policyId route param is a non-empty string. +// @POST Returns { task, runs, total } or redirects to 404 on missing task. +import { getValidationTask, getValidationRuns } from '$lib/api.js'; +import { error } from '@sveltejs/kit'; +import type { PageLoad } from './$types'; + +interface TaskResponse { + task?: Record; + [key: string]: unknown; +} + +interface RunsResponse { + results?: unknown[]; + items?: unknown[]; + total?: number; + count?: number; +} + +interface LoadResult { + task: Record; + runs: unknown[]; + total: number; +} + +export const load: PageLoad = async ({ params }): Promise => { + const { policyId } = params as { policyId: string }; + + const [taskRes, runsRes] = await Promise.all([ + getValidationTask(policyId).catch(() => null) as Promise, + getValidationRuns(policyId, { page: 1, page_size: 20 }).catch(() => ({ results: [], total: 0 })) as Promise, + ]); + + if (!taskRes) { + error(404, 'Task not found'); + } + + const task = (taskRes as TaskResponse).task || taskRes; + const runs = Array.isArray(runsRes) ? runsRes : ((runsRes as RunsResponse)?.results || (runsRes as RunsResponse)?.items || []); + const total = (runsRes as RunsResponse)?.total || (runsRes as RunsResponse)?.count || runs.length; + + return { task, runs, total }; +}; +// #endregion ValidationTaskDetailPageLoad diff --git a/frontend/src/routes/validation-tasks/[policyId]/edit/+page.js b/frontend/src/routes/validation-tasks/[policyId]/edit/+page.js deleted file mode 100644 index dd52c4b8..00000000 --- a/frontend/src/routes/validation-tasks/[policyId]/edit/+page.js +++ /dev/null @@ -1,32 +0,0 @@ -// #region ValidationTaskEditPageLoad [C:2] [TYPE Function] [SEMANTICS validation-tasks,edit,load,task,providers] -// @BRIEF Load function for the edit validation task page — fetches existing task data and LLM providers list. -// @RELATION CALLS -> [ApiModule.getValidationTask] -// @RELATION CALLS -> [ApiModule.getLlmProviders] -// @POST Returns { task, llmProviders } or redirects to 404 on missing task. -import { api } from '$lib/api.js'; -import { error } from '@sveltejs/kit'; - -/** @type {import('./$types').PageLoad} */ -export async function load({ params }) { - const { policyId } = params; - - const [taskResult, providersResult] = await Promise.all([ - api.getValidationTask(policyId).catch(() => null), - api.getLlmProviders().catch(() => []), - ]); - - if (!taskResult) { - error(404, 'Task not found'); - } - - const task = taskResult.task || taskResult; - const providers = Array.isArray(providersResult) - ? providersResult - : (providersResult?.providers ?? providersResult?.results ?? []); - - return { - task, - llmProviders: providers, - }; -} -// #endregion ValidationTaskEditPageLoad diff --git a/frontend/src/routes/validation-tasks/[policyId]/edit/+page.ts b/frontend/src/routes/validation-tasks/[policyId]/edit/+page.ts new file mode 100644 index 00000000..1443d413 --- /dev/null +++ b/frontend/src/routes/validation-tasks/[policyId]/edit/+page.ts @@ -0,0 +1,45 @@ +// #region ValidationTaskEditPageLoad [C:2] [TYPE Function] [SEMANTICS validation-tasks,edit,load,task,providers] +// @BRIEF Load function for the edit validation task page — fetches existing task data and LLM providers list. +// @RELATION CALLS -> [ApiModule.getValidationTask] +// @RELATION CALLS -> [ApiModule.getLlmProviders] +// @POST Returns { task, llmProviders } or redirects to 404 on missing task. +import { api } from '$lib/api.js'; +import { error } from '@sveltejs/kit'; +import type { PageLoad } from './$types'; + +interface TaskResponse { + task?: Record; + [key: string]: unknown; +} + +interface ProvidersResponse { + providers?: unknown[]; + results?: unknown[]; + [key: string]: unknown; +} + +interface LoadResult { + task: Record; + llmProviders: unknown[]; +} + +export const load: PageLoad = async ({ params }): Promise => { + const { policyId } = params as { policyId: string }; + + const [taskResult, providersResult] = await Promise.all([ + api.getValidationTask(policyId).catch(() => null) as Promise, + api.getLlmProviders().catch(() => []) as Promise, + ]); + + if (!taskResult) { + error(404, 'Task not found'); + } + + const task = (taskResult as TaskResponse).task || taskResult; + const providers = Array.isArray(providersResult) + ? providersResult + : ((providersResult as ProvidersResponse)?.providers ?? (providersResult as ProvidersResponse)?.results ?? []); + + return { task, llmProviders: providers }; +}; +// #endregion ValidationTaskEditPageLoad diff --git a/frontend/src/routes/validation-tasks/[policyId]/runs/[runId]/+page.js b/frontend/src/routes/validation-tasks/[policyId]/runs/[runId]/+page.js deleted file mode 100644 index 2365645c..00000000 --- a/frontend/src/routes/validation-tasks/[policyId]/runs/[runId]/+page.js +++ /dev/null @@ -1,32 +0,0 @@ -// #region ValidationRunDetailPageLoad [C:2] [TYPE Module] [SEMANTICS validation, run, detail, report, load] -// @BRIEF SvelteKit load function for the validation run detail report page — fetches full run results including dashboards, issues, screenshots, dataset health, and logs. -// @RELATION CALLS -> [ValidationApi] -// @PRE policyId and runId route params are non-empty strings. -// @POST Returns { run } or redirects to 404 on missing run. -import { getValidationRunDetail } from '$lib/api.js'; -import { error } from '@sveltejs/kit'; - -/** @type {import('./$types').PageLoad} */ -export async function load({ params }) { - const { policyId, runId } = params; - - const runDetail = await getValidationRunDetail(policyId, runId).catch(() => null); - - if (!runDetail) { - error(404, 'Run not found'); - } - - // Normalise shape: merge records into run as dashboards for the report page - const run = runDetail.run || runDetail; - const records = runDetail.records || []; - - // Normalise field names from backend API shape to frontend template keys - run.dashboards = records.map((rec) => ({ - ...rec, - screenshots: rec.screenshots || rec.screenshot_paths || rec.tab_screenshots || [], - logs_sent: rec.logs_sent || rec.logs_sent_to_llm || [], - })); - - return { run }; -} -// #endregion ValidationRunDetailPageLoad diff --git a/frontend/src/routes/validation-tasks/[policyId]/runs/[runId]/+page.svelte b/frontend/src/routes/validation-tasks/[policyId]/runs/[runId]/+page.svelte index f4adb71c..ca9d5089 100644 --- a/frontend/src/routes/validation-tasks/[policyId]/runs/[runId]/+page.svelte +++ b/frontend/src/routes/validation-tasks/[policyId]/runs/[runId]/+page.svelte @@ -8,6 +8,9 @@ + + +