diff --git a/backend/src/plugins/llm_analysis/plugin.py b/backend/src/plugins/llm_analysis/plugin.py index 3ad7de3d..f328b3e6 100644 --- a/backend/src/plugins/llm_analysis/plugin.py +++ b/backend/src/plugins/llm_analysis/plugin.py @@ -104,27 +104,31 @@ def _ensure_json_prompt(prompt: str | None) -> str: # #region _update_run_status [TYPE Function] -# @BRIEF Update ValidationRun status based on its records' aggregate status. -# @PRE db session is active, run_id may be None (v1 records or direct trigger). -# @POST If run_id is set, ValidationRun.status is updated to reflect the worst record status. +# @BRIEF Aggregate ValidationRecord statuses into the ValidationRun. +# Only marks run as finished when ALL dashboards have a record. +# @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. def _update_run_status(db, run_id: str | None) -> None: - """Aggregate ValidationRecord statuses into the ValidationRun.""" 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() - if not records: + 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 = db.query(VRun).filter(VRun.id == run_id).first() - if run: - run.status = worst.status - from datetime import datetime, timezone - run.finished_at = datetime.now(timezone.utc) - db.commit() + run.status = worst.status + from datetime import datetime, timezone + run.finished_at = datetime.now(timezone.utc) + db.commit() except Exception: pass # #endregion _update_run_status @@ -235,59 +239,72 @@ class DashboardValidationPlugin(PluginBase): "Please open LLM provider settings and save a real API key (not masked placeholder)." ) - # Resolve dashboard_id from v2 list or v1 single string - dashboard_ids = params.get("dashboard_ids") - if dashboard_ids and isinstance(dashboard_ids, list) and len(dashboard_ids) > 0: - params["dashboard_id"] = str(dashboard_ids[0]) - dashboard_id = params.get("dashboard_id") - if not dashboard_id: + # Resolve dashboard_ids from v2 list or v1 single string + all_dashboard_ids: list[str] = [] + raw_ids = params.get("dashboard_ids") + if raw_ids and isinstance(raw_ids, list) and len(raw_ids) > 0: + all_dashboard_ids = [str(did) for did in raw_ids] + single_id = params.get("dashboard_id") + if single_id and single_id not in all_dashboard_ids: + all_dashboard_ids.append(str(single_id)) + if not all_dashboard_ids: raise ValueError("No dashboard_id provided in params (dashboard_ids list is empty or missing)") - # Re-parse URL sources per FR-057 - dashboard_url = params.get("dashboard_url") - if dashboard_url: - try: - from ...core.utils.superset_context_extractor import SupersetContextExtractor - - client = SupersetClient(env) - extractor = SupersetContextExtractor(environment=env, client=client) - parsed = extractor.parse_superset_link(dashboard_url) - - if parsed.dashboard_id: - params["dashboard_id"] = str(parsed.dashboard_id) - - # Store parsed context for tab/filter navigation - params["parsed_context"] = { - "dashboard_id": parsed.dashboard_id, - "native_filters": parsed.imported_filters if hasattr(parsed, "imported_filters") else None, - "active_tabs": [], - "anchor": None, - } - - log.info(f"[URL re-parse] Resolved dashboard_id={parsed.dashboard_id} from URL") - - except Exception as e: - log.warning(f"[URL re-parse] Failed to parse URL: {e}") - # Mark source as failed — task will skip this dashboard - params["source_invalid"] = True - params["source_error"] = str(e) - screenshot_enabled = params.get("screenshot_enabled", True) + if screenshot_enabled and not bool(db_provider.is_multimodal): + raise ValueError( + "Dashboard validation with screenshot_enabled=True requires " + "a multimodal model (image input support)." + ) - if screenshot_enabled: - # Path A requires a multimodal provider for image input - if not bool(db_provider.is_multimodal): - raise ValueError( - "Dashboard validation with screenshot_enabled=True requires " - "a multimodal model (image input support)." + # Process each dashboard sequentially, collecting results + results: list[dict[str, Any]] = [] + 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 + + # Re-parse URL sources per FR-057 + dashboard_url = params.get("dashboard_url") + if dashboard_url: + try: + from ...core.utils.superset_context_extractor import SupersetContextExtractor + + client = SupersetClient(env) + extractor = SupersetContextExtractor(environment=env, client=client) + parsed = extractor.parse_superset_link(dashboard_url) + + if parsed.dashboard_id: + params["dashboard_id"] = str(parsed.dashboard_id) + + # Store parsed context for tab/filter navigation + params["parsed_context"] = { + "dashboard_id": parsed.dashboard_id, + "native_filters": parsed.imported_filters if hasattr(parsed, "imported_filters") else None, + "active_tabs": [], + "anchor": None, + } + + log.info(f"[URL re-parse] Resolved dashboard_id={parsed.dashboard_id} from URL") + + except Exception as e: + log.warning(f"[URL re-parse] Failed to parse URL: {e}") + # Mark source as failed — task will skip this dashboard + 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 ) - return await self._execute_path_a( - params, context, db, db_provider, api_key, env, config_mgr, log - ) - else: - return await self._execute_path_b( - 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) + + # After ALL dashboards processed, update run status to reflect completion + _update_run_status(db, params.get("run_id")) + return {"dashboards": results, "total": len(results)} finally: db.close() @@ -466,9 +483,6 @@ class DashboardValidationPlugin(PluginBase): db.add(db_record) db.commit() - # Update ValidationRun status after record is persisted - _update_run_status(db, params.get("run_id")) - # 7. Notification on failure (US1 / FR-015) try: policy_id = params.get("policy_id") @@ -679,9 +693,6 @@ class DashboardValidationPlugin(PluginBase): db.add(db_record) db.commit() - # Update ValidationRun status after record is persisted - _update_run_status(db, params.get("run_id")) - # 8. Notification on failure (US1 / FR-015) try: policy_id = params.get("policy_id") diff --git a/backend/src/services/__tests__/test_llm_plugin_persistence.py b/backend/src/services/__tests__/test_llm_plugin_persistence.py index 63e02f79..49e0c17c 100644 --- a/backend/src/services/__tests__/test_llm_plugin_persistence.py +++ b/backend/src/services/__tests__/test_llm_plugin_persistence.py @@ -218,7 +218,9 @@ async def test_dashboard_validation_plugin_persists_task_and_environment_ids( context=context, ) - assert result["environment_id"] == "env-42" + # Plugin now returns a batch result with all dashboards + assert result["total"] == 1 + assert result["dashboards"][0]["environment_id"] == "env-42" assert fake_db.committed is True assert fake_db.closed is True assert fake_db.added is not None