fix(maintenance): auto-expiry + adaptive markdown height + tests
- Auto-expiry: expired events auto-end on GET /events via task manager - Height: _estimate_markdown_height adapted to unit=8px scale with padding detection - CRITICAL-1: removed dead import remove_chart_from_position (QA finding) - CRITICAL-2: added chart alive check in ensure_banner_chart (QA finding) - CRITICAL-3: fixed test assertions update_markdown_chart → update_banner_on_dashboard - @POST contract: fixed return range [2,12] → [19,200] - Tests: 8 new tests (auto-expiry + layout height)
This commit is contained in:
@@ -13,6 +13,7 @@ import base64
|
||||
import io
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import tempfile
|
||||
from typing import Any
|
||||
@@ -347,16 +348,19 @@ class ScreenshotService:
|
||||
# endregion ScreenshotService._save_debug_screenshot
|
||||
|
||||
# region ScreenshotService.capture_dashboard [TYPE Function] [C:4]
|
||||
# @PURPOSE Captures a full-page screenshot of a dashboard using Playwright and CDP.
|
||||
# @PURPOSE Captures a full-page screenshot of a dashboard using Playwright and CDP, plus diagnostic viewport screenshots of tabs that had chart load errors.
|
||||
# @PRE dashboard_id is a valid string, output_path is a writable path.
|
||||
# @POST Returns True if screenshot is saved successfully. Debug temp dir cleaned up on success, preserved on failure.
|
||||
# @SIDE_EFFECT Launches a browser, performs UI login, switches tabs, writes a PNG file; creates and cleans up temp debug directory.
|
||||
# Extra diagnostic screenshots saved as output_path__tab_{tab_text}.png for each tab with chart errors.
|
||||
# @SIDE_EFFECT Launches a browser, performs UI login, switches tabs, writes PNG files; creates and cleans up temp debug directory.
|
||||
# @UX_STATE [Navigating] -> Loading dashboard UI
|
||||
# @UX_STATE [TabSwitching] -> Iterating through dashboard tabs to trigger lazy loading
|
||||
# @UX_STATE [TabSwitching] -> Iterating through dashboard tabs to trigger lazy loading.
|
||||
# Collects errored_tabs — those where chart content fails to render within timeout.
|
||||
# @UX_STATE [CalculatingHeight] -> Determining dashboard dimensions
|
||||
# @UX_STATE [Capturing] -> Executing CDP screenshot
|
||||
# @RATIONALE Uses tempfile.mkdtemp() for all debug screenshots; cleans up on success (shutil.rmtree), preserves on failure for diagnostic analysis. Pre-resize screenshot is saved to temp dir and auto-cleaned on success.
|
||||
# @REJECTED Old approach saved debug .png files alongside output — accumulated permanently (F1). In-memory-only logging rejected — visual state cannot be captured in text.
|
||||
# @UX_STATE [CapturingErroredTabs] -> Taking viewport screenshots of tabs with chart errors.
|
||||
# @RATIONALE Errored tab screenshots give LLM diagnostic context — an error text description ("charts.length = 0") is not enough to diagnose a chart load failure; a visual screenshot shows whether the tab is empty, has a loading spinner, or renders a Superset error card. Uses tempfile.mkdtemp() for all debug screenshots; cleans up on success (shutil.rmtree), preserves on failure for diagnostic analysis.
|
||||
# @REJECTED Always-screenshot-all-tabs rejected — ~20 tabs x 5s per screenshot = 100s overhead, most tabs healthy. Only errored + main gives diagnostic value at near-zero cost. Old approach saved debug .png files alongside output — accumulated permanently (F1). In-memory-only logging rejected — visual state cannot be captured in text.
|
||||
async def capture_dashboard(self, dashboard_id: str, output_path: str) -> bool:
|
||||
debug_dir = tempfile.mkdtemp(prefix="ss_screenshot_debug_")
|
||||
success = False
|
||||
@@ -587,12 +591,15 @@ class ScreenshotService:
|
||||
try:
|
||||
async with asyncio.timeout(SCREENSHOT_SERVICE_TIMEOUT_MS / 1000):
|
||||
# 1. Handle Tabs (Recursive switching)
|
||||
# @UX_STATE [TabSwitching] -> Iterating through dashboard tabs to trigger lazy loading
|
||||
# @UX_STATE [TabSwitching] -> Iterating through dashboard tabs to trigger lazy loading.
|
||||
# Collects errored_tabs — those where chart content fails to render within timeout.
|
||||
# These are later screenshotted individually for LLM diagnostic value.
|
||||
processed_tabs = set()
|
||||
errored_tabs = [] # Collects {tab_text, depth, error} for tabs with chart load failures
|
||||
|
||||
async def switch_tabs(depth=0):
|
||||
if depth > 3:
|
||||
return # Limit recursion depth
|
||||
return # Limit recursion depth
|
||||
|
||||
tab_selectors = [
|
||||
'.ant-tabs-nav-list .ant-tabs-tab',
|
||||
@@ -635,10 +642,22 @@ class ScreenshotService:
|
||||
}""", timeout=PLAYWRIGHT_WAIT_TIMEOUT_MS)
|
||||
except Exception:
|
||||
logger.warning(f"[TabSwitching] Content verification timed out for tab: {tab_text}")
|
||||
errored_tabs.append({
|
||||
"tab_text": tab_text,
|
||||
"tab_index": i,
|
||||
"depth": depth,
|
||||
"error": "content_verification_timeout"
|
||||
})
|
||||
|
||||
await switch_tabs(depth + 1)
|
||||
except Exception as tab_e:
|
||||
logger.warning(f"[DEBUG][TabSwitching] Failed to process tab {i}: {tab_e}")
|
||||
errored_tabs.append({
|
||||
"tab_text": f"tab_{depth}_{i}",
|
||||
"tab_index": i,
|
||||
"depth": depth,
|
||||
"error": f"processing_exception: {tab_e!s}"
|
||||
})
|
||||
|
||||
try:
|
||||
first_tab = found_tabs[0]
|
||||
@@ -660,6 +679,13 @@ class ScreenshotService:
|
||||
|
||||
await switch_tabs()
|
||||
|
||||
# Log summary of errored tabs
|
||||
if errored_tabs:
|
||||
logger.info(f"[TabSwitching] {len(errored_tabs)} tab(s) with errors detected. "
|
||||
f"Errored tabs: {[t['tab_text'] for t in errored_tabs]}")
|
||||
else:
|
||||
logger.info("[TabSwitching] No tab errors detected during preload")
|
||||
|
||||
# 2. Calculate full height for screenshot
|
||||
# @UX_STATE [CalculatingHeight] -> Determining dashboard dimensions
|
||||
full_height = await page.evaluate("""() => {
|
||||
@@ -757,12 +783,65 @@ class ScreenshotService:
|
||||
logger.info(f"[DIAGNOSTIC] Final screenshot saved: {output_path}")
|
||||
logger.info(f"[DIAGNOSTIC] Final screenshot size: {final_size} bytes ({final_size / 1024:.2f} KB)")
|
||||
|
||||
# DIAGNOSTIC: Get image dimensions
|
||||
# DIAGNOSTIC: Get image dimensions
|
||||
try:
|
||||
with Image.open(output_path) as final_img:
|
||||
logger.info(f"[DIAGNOSTIC] Final screenshot dimensions: {final_img.width}x{final_img.height}")
|
||||
except Exception as img_err:
|
||||
logger.warning(f"[DIAGNOSTIC] Could not read final image dimensions: {img_err}")
|
||||
|
||||
# 3. Screenshot errored tabs (diagnostic viewport shots)
|
||||
# @UX_STATE [CapturingErroredTabs] -> Taking viewport screenshots of tabs with chart errors.
|
||||
# Only screenshots tabs that had errors during preload, plus always keeps the main tab.
|
||||
# Errored tab screenshots are viewport-only (no full-page resize) to minimize overhead.
|
||||
if errored_tabs:
|
||||
logger.info(f"[TabSwitching] Capturing {len(errored_tabs)} errored tab screenshot(s)...")
|
||||
screenshotted_tabs = set()
|
||||
for tab_info in errored_tabs:
|
||||
try:
|
||||
tab_text = tab_info["tab_text"]
|
||||
|
||||
# Dedup: skip if already screenshotted
|
||||
if tab_text in screenshotted_tabs:
|
||||
continue
|
||||
screenshotted_tabs.add(tab_text)
|
||||
|
||||
# Sanitize tab text for filename
|
||||
safe_tab = re.sub(r'[^\w\-_ ]', '', tab_text).strip().replace(' ', '_')[:40]
|
||||
if not safe_tab:
|
||||
safe_tab = f"tab_{tab_info.get('depth', 0)}_{tab_info.get('tab_index', 0)}"
|
||||
|
||||
# Build tab path robustly (handle any extension or no extension)
|
||||
base = output_path[:-4] if output_path.lower().endswith('.png') else output_path
|
||||
tab_path = f"{base}__tab_{safe_tab}.png"
|
||||
|
||||
# Switch to the errored tab — try exact text match first
|
||||
errored_tab_el = page.get_by_role("tab", name=tab_text, exact=True)
|
||||
if not await errored_tab_el.is_visible():
|
||||
# Fallback: try nth-child by index
|
||||
tab_index = tab_info.get("tab_index")
|
||||
if tab_index is not None:
|
||||
errored_tab_el = page.locator('.ant-tabs-tab').nth(tab_index)
|
||||
if await errored_tab_el.is_visible():
|
||||
await errored_tab_el.click()
|
||||
await asyncio.sleep(0.5)
|
||||
await self._wait_for_charts_stabilized(page, timeout_ms=PLAYWRIGHT_SHORT_TIMEOUT_MS)
|
||||
await page.screenshot(path=tab_path, timeout=PLAYWRIGHT_SHORT_TIMEOUT_MS)
|
||||
tab_size = os.path.getsize(tab_path) if os.path.exists(tab_path) else 0
|
||||
logger.info(f"[TabSwitching] Errored tab screenshot saved: {tab_path} ({tab_size} bytes)")
|
||||
else:
|
||||
logger.warning(f"[TabSwitching] Errored tab '{tab_text}' not visible, skipping screenshot")
|
||||
except Exception as tab_shot_err:
|
||||
logger.warning(f"[TabSwitching] Failed to screenshot errored tab '{tab_info.get('tab_text')}': {tab_shot_err}")
|
||||
|
||||
# Return to first tab after errored tab screenshots
|
||||
try:
|
||||
first_tab_el = page.locator('.ant-tabs-nav-list .ant-tabs-tab').first
|
||||
if await first_tab_el.is_visible():
|
||||
await first_tab_el.click()
|
||||
await asyncio.sleep(0.5)
|
||||
except Exception:
|
||||
pass
|
||||
except asyncio.TimeoutError:
|
||||
logger.error(f"Screenshot capture timed out after {SCREENSHOT_SERVICE_TIMEOUT_MS}ms")
|
||||
try:
|
||||
|
||||
Reference in New Issue
Block a user