From 6433d9bea1980829c9bff3f26a1825ccc05266ea Mon Sep 17 00:00:00 2001 From: Damian Sova Date: Fri, 14 Aug 2026 13:44:12 +0200 Subject: [PATCH 1/4] fix(supervisor): handle JSON parse edge case in ci_budget tracker When gh api returns non-dict JSON (e.g., null from a parse error that produced valid JSON), the tracker fell through to a KeyError instead of returning zero budget. Consolidate the bad_json path so non-dict values are caught by the existing isinstance guard. Add test covering the exception path when resolve_gh_env raises. --- sova/supervisor/ci_budget.py | 3 +-- tests/test_ci_budget.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/sova/supervisor/ci_budget.py b/sova/supervisor/ci_budget.py index 977963d0..e4e83130 100644 --- a/sova/supervisor/ci_budget.py +++ b/sova/supervisor/ci_budget.py @@ -95,8 +95,7 @@ async def _fetch(self, repo: str, github_user: str) -> CIBudget: try: data = json.loads(result.stdout) except (json.JSONDecodeError, TypeError): - log.warning("ci_budget.bad_json", repo=repo) - return _zero_budget() + data = None if not isinstance(data, dict): log.warning("ci_budget.bad_json", repo=repo) diff --git a/tests/test_ci_budget.py b/tests/test_ci_budget.py index fd315adc..203d8868 100644 --- a/tests/test_ci_budget.py +++ b/tests/test_ci_budget.py @@ -575,6 +575,20 @@ async def test_no_plan_object_defaults_to_free(self) -> None: assert minutes == 2000 + @pytest.mark.asyncio + async def test_resolve_gh_env_exception_continues(self) -> None: + tracker = CIBudgetTracker() + mock_result = MagicMock() + mock_result.success = True + mock_result.stdout = '{"total_minutes_used": 200, "included_minutes": 3000}' + + with patch("sova.utils.shell.run", new_callable=AsyncMock, return_value=mock_result): + with patch("sova.utils.gh.resolve_gh_env", new_callable=AsyncMock, side_effect=RuntimeError("auth failed")): + budget = await tracker.get_budget("owner/repo", "user") + + assert budget.total == 3000 + assert budget.used == 200 + class TestCIBudgetTrackerFactory: def test_same_identity_returns_same_instance(self) -> None: From ac71a22c247b804df92f7c628d145e3b2f97fab6 Mon Sep 17 00:00:00 2001 From: Damian Sova Date: Fri, 14 Aug 2026 13:44:19 +0200 Subject: [PATCH 2/4] fix(commands): correct issue_number type in merge-queue marker Remove stale single-quotes around in the merge-queue marker JSON snippet so it serializes as an integer, matching the MergeQueueEntry model's integer field. --- .claude/commands/approve-merge.md | 2 +- .claude/commands/integrate-pr.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/commands/approve-merge.md b/.claude/commands/approve-merge.md index 5aaf08e7..b815fc28 100644 --- a/.claude/commands/approve-merge.md +++ b/.claude/commands/approve-merge.md @@ -116,7 +116,7 @@ If merge queue is detected (or forced via config): - Write a merge queue marker file so the dashboard can track the PR: ```bash mkdir -p .claude/agent-control - python3 -c "import json; print(json.dumps({'pr_number': , 'repo': '', 'issue_number': '', 'branch_name': ''}))" > .claude/agent-control/merge-queue.json + python3 -c "import json; print(json.dumps({'pr_number': , 'repo': '', 'issue_number': , 'branch_name': ''}))" > .claude/agent-control/merge-queue.json ``` - Proceed to queue polling (step 3b) diff --git a/.claude/commands/integrate-pr.md b/.claude/commands/integrate-pr.md index a8e2f5e2..54175949 100644 --- a/.claude/commands/integrate-pr.md +++ b/.claude/commands/integrate-pr.md @@ -171,7 +171,7 @@ If merge queue is detected: - If enqueued, write a merge queue marker file so the dashboard can track the PR: ```bash mkdir -p .claude/agent-control - python3 -c "import json; print(json.dumps({'pr_number': , 'repo': '', 'issue_number': '', 'branch_name': ''}))" > .claude/agent-control/merge-queue.json + python3 -c "import json; print(json.dumps({'pr_number': , 'repo': '', 'issue_number': , 'branch_name': ''}))" > .claude/agent-control/merge-queue.json ``` - Then poll merge queue status via GraphQL every `merge_queue_poll_interval` seconds (default 30) - On MERGED: proceed to Phase 6. If `delete_branch = true`, delete remote branch via GitHub API From 9955b96b55a75152bdab817406d7ee7a56d4763e Mon Sep 17 00:00:00 2001 From: Damian Sova Date: Fri, 14 Aug 2026 13:44:36 +0200 Subject: [PATCH 3/4] chore(core): parallel test execution and fixture repairs Add pytest-xdist for parallel test runs (-n auto). Fix flaky fixtures: hoist create_app import to module level in test_dashboard, use MagicMock for sync write_handoff_file in test_roles, mock read_pid_file in test_scheduler server status test. Update step count (29 -> 31) in architecture.md and watchdog docstring for auto-retry path. --- .claude/rules/architecture.md | 2 +- Makefile | 2 +- pyproject.toml | 1 + sova/supervisor/watchdog.py | 4 +- tests/test_dashboard.py | 17 +-------- tests/test_roles.py | 72 +++++++++++++++++------------------ tests/test_scheduler.py | 2 - 7 files changed, 42 insertions(+), 58 deletions(-) diff --git a/.claude/rules/architecture.md b/.claude/rules/architecture.md index e1eaa230..96deb685 100644 --- a/.claude/rules/architecture.md +++ b/.claude/rules/architecture.md @@ -14,7 +14,7 @@ SOVA has four main components: - `core/state.py` -- 19-state TaskStatus StrEnum with transition validation - `core/context.py` -- ExecutionContext dataclass threading state through steps - `core/output.py` -- OutputWriter for per-run DB-backed output persistence, read_lines, retention cleanup -- `core/steps/` -- 29 BaseStep implementations with execute/validate_output/can_skip. Four pipeline variants: +- `core/steps/` -- 31 BaseStep implementations with execute/validate_output/can_skip. Four pipeline variants: - **Developer pipeline** (16 steps): sync -> assess -> create_worktree -> capture_baseline -> develop -> simplify -> self_review -> commit -> validate -> push -> create_pr -> wait_for_external_reviews -> address_external_findings -> monitor_ci -> extract_memory -> handoff_to_reviewer - **Address-review pipeline** (10 steps): ensure_worktree -> rebase -> address_review -> rearrange_commits -> validate -> push -> monitor_ci -> resolve_external_reviews -> extract_memory -> handoff_to_user - **Researcher pipeline** (4 steps): fetch_task -> research -> spec -> extract_memory diff --git a/Makefile b/Makefile index a9d1f004..33e9d8d5 100644 --- a/Makefile +++ b/Makefile @@ -33,7 +33,7 @@ test-bash: lint-bash ## Validate bash scripts (shellcheck + --help) done test-py: ## Run pytest suite (excludes runtime/stress/chaos) - $(PYTEST) tests/ -v -m "not runtime and not stress and not chaos" + $(PYTEST) tests/ -v -m "not runtime and not stress and not chaos" -n auto test-runtime: ## Run runtime, stress, and chaos tests (manual) $(PYTEST) tests/ -v -m "runtime or stress or chaos" --timeout=120 diff --git a/pyproject.toml b/pyproject.toml index 9db0205e..254dfecb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,6 +55,7 @@ dev = [ "pytest-asyncio>=0.24", "pytest-cov>=6.0", "pytest-timeout>=2.3", + "pytest-xdist>=3.0", "ruff>=0.4", "respx>=0.22", ] diff --git a/sova/supervisor/watchdog.py b/sova/supervisor/watchdog.py index 4ecabfc9..c6f7d7d6 100644 --- a/sova/supervisor/watchdog.py +++ b/sova/supervisor/watchdog.py @@ -5,8 +5,8 @@ (pipeline not adopted, no output, step timeout, zombie process), and takes corrective action (warn via feed event, kill via stop_agent). -Killed agents flow through the existing _wait_and_finalize path. -The watchdog never independently retries. +Killed agents flow through the existing _wait_and_finalize -> _schedule_retry() +path for auto-retry. The watchdog never independently retries. """ from __future__ import annotations diff --git a/tests/test_dashboard.py b/tests/test_dashboard.py index 381f7850..399edbab 100644 --- a/tests/test_dashboard.py +++ b/tests/test_dashboard.py @@ -12,6 +12,7 @@ from httpx import ASGITransport, AsyncClient from sqlalchemy.ext.asyncio import AsyncSession +from sova.dashboard.app import create_app from sova.db.models import CostRecord, Memory, StepExecution, TaskRun from sova.db.session import close_db, get_session, init_db @@ -150,7 +151,6 @@ async def seed_data(session: AsyncSession): @pytest.fixture async def client(): - from sova.dashboard.app import create_app app = create_app(multi_project=False) transport = ASGITransport(app=app) @@ -3735,8 +3735,6 @@ async def multi_client(self, tmp_path): registry.register_project(p1, slug="alpha") registry.register_project(p2, slug="beta") - from sova.dashboard.app import create_app - app = create_app(multi_project=True) transport = ASGITransport(app=app) async with AsyncClient(transport=transport, base_url="http://test") as ac: @@ -13859,8 +13857,6 @@ def test_websocket_connect_and_receive_status_update(self) -> None: """WebSocket endpoint accepts connection and sends status_update with runs.""" from starlette.testclient import TestClient - from sova.dashboard.app import create_app - app = create_app(multi_project=False) client = TestClient(app) with client.websocket_connect("/api/ws/agents/status") as ws: @@ -13872,7 +13868,6 @@ def test_websocket_client_disconnect(self) -> None: """Verify connection is removed from manager after client disconnects.""" from starlette.testclient import TestClient - from sova.dashboard.app import create_app from sova.dashboard.routers.agents import _ws_manager app = create_app(multi_project=False) @@ -13887,8 +13882,6 @@ def test_websocket_sequential_clients(self) -> None: """Sequential clients can each connect and receive updates.""" from starlette.testclient import TestClient - from sova.dashboard.app import create_app - app = create_app(multi_project=False) # Use separate TestClient instances to avoid threading deadlock # when nesting websocket_connect context managers on the same client. @@ -13907,8 +13900,6 @@ def test_websocket_error_handling(self) -> None: from starlette.testclient import TestClient - from sova.dashboard.app import create_app - app = create_app(multi_project=False) client = TestClient(app) with ( @@ -14013,8 +14004,6 @@ async def install_client(self, tmp_path, monkeypatch): lambda: project_dir, ) - from sova.dashboard.app import create_app - app = create_app(project_dir=project_dir) transport = ASGITransport(app=app) async with AsyncClient(transport=transport, base_url="http://test") as ac: @@ -14776,7 +14765,6 @@ async def test_resume_clears_handoff(self) -> None: async def test_resume_endpoint_404(self) -> None: """The router endpoint returns 404 for missing run.""" - from sova.dashboard.app import create_app app = create_app() transport = ASGITransport(app=app) @@ -14786,7 +14774,6 @@ async def test_resume_endpoint_404(self) -> None: async def test_resume_endpoint_409(self) -> None: """The router endpoint returns 409 for wrong status.""" - from sova.dashboard.app import create_app async with await get_session() as session: async with session.begin(): @@ -14873,8 +14860,6 @@ async def test_resume_endpoint_500_on_spawn_error(self) -> None: """The router returns 500 for generic spawn errors (not 404/409).""" from unittest.mock import AsyncMock, patch - from sova.dashboard.app import create_app - async with await get_session() as session: async with session.begin(): run = TaskRun( diff --git a/tests/test_roles.py b/tests/test_roles.py index acdb3bbd..204d3a65 100644 --- a/tests/test_roles.py +++ b/tests/test_roles.py @@ -6,7 +6,7 @@ import os from decimal import Decimal from pathlib import Path -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, MagicMock import pytest @@ -751,7 +751,7 @@ async def test_execute_reviews_pr(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -782,7 +782,7 @@ async def test_execute_discovers_pr_when_not_provided(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -816,7 +816,7 @@ async def test_reviewer_discovers_branch_when_pr_number_preset(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -852,7 +852,7 @@ async def test_reviewer_propagates_branch_name_in_handoff(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock) as mock_db_handoff, - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): await role.execute(ctx) @@ -907,7 +907,7 @@ async def test_execute_branch_discovery_failure_non_fatal(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -934,7 +934,7 @@ async def test_execute_extract_memory_failure_non_fatal(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), patch( "sova.knowledge.extraction.extract_memories", new_callable=AsyncMock, @@ -971,7 +971,7 @@ async def test_execute_handoff_db_failure_non_fatal(self) -> None: new_callable=AsyncMock, side_effect=RuntimeError("DB write failed"), ), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -1029,7 +1029,7 @@ async def test_execute_review_rationale_failure_non_fatal(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), patch( "sova.core.steps._spec_helpers.append_spec_section", side_effect=OSError("spec write failed"), @@ -1083,7 +1083,7 @@ async def test_execute_posts_inline_review_comments(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["foo.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -1119,7 +1119,7 @@ async def test_execute_falls_back_to_comment_on_review_failure(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -1170,7 +1170,7 @@ async def fail_with_inline_succeed_without(pr_number, body, event, comments): patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["foo.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -1216,7 +1216,7 @@ async def test_reviewer_falls_back_to_comment_when_body_only_also_fails(self) -> patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["foo.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -1281,7 +1281,7 @@ async def _capture_invoke(prompt, **kwargs): patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, side_effect=_capture_invoke), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -1324,7 +1324,7 @@ async def _capture_invoke(prompt, **kwargs): patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, side_effect=_capture_invoke), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -2998,7 +2998,7 @@ async def test_successful_review_with_findings(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["foo.py", "bar.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -3027,7 +3027,7 @@ async def test_review_no_findings(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -3052,7 +3052,7 @@ async def test_llm_failure_graceful_fallback(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, side_effect=RuntimeError("LLM unavailable")), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -3095,7 +3095,7 @@ async def test_large_diff_chunking(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py", "b.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, side_effect=side) as mock_invoke, patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -3124,7 +3124,7 @@ async def test_handoff_address_review_when_actionable(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock) as mock_db_handoff, - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): role = ReviewerRole() await role.execute(ctx) @@ -3158,7 +3158,7 @@ async def test_handoff_all_findings_actionable(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock) as mock_db_handoff, - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): role = ReviewerRole() await role.execute(ctx) @@ -3193,7 +3193,7 @@ async def test_handoff_auto_execute_disabled_by_config(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): role = ReviewerRole() await role.execute(ctx) @@ -3221,7 +3221,7 @@ async def test_handoff_approve_when_zero_findings(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock) as mock_db_handoff, - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): role = ReviewerRole() await role.execute(ctx) @@ -3294,7 +3294,7 @@ async def test_clears_current_step_sentinel(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -3334,7 +3334,7 @@ async def test_clear_current_step_db_failure_non_fatal(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), patch("sova.roles.reviewer.get_session", side_effect=OSError("DB connection refused")), ): role = ReviewerRole() @@ -3372,7 +3372,7 @@ async def test_no_clear_without_task_run_id(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), patch("sova.roles.reviewer.read_handoff_file", return_value=None), patch("sova.roles.reviewer.get_session", new_callable=AsyncMock), patch.object(ReviewerRole, "_clear_current_step", mock_clear_step), @@ -3924,7 +3924,7 @@ async def test_write_verdict_label_skipped_when_no_issue(self) -> None: async def test_write_handoff_db_exception_non_fatal(self) -> None: """_write_handoff handles DB write failure gracefully (lines 733-734).""" - from unittest.mock import patch + from unittest.mock import AsyncMock, MagicMock, patch from sova.roles.reviewer import ReviewerRole, ReviewResult @@ -3935,7 +3935,7 @@ async def test_write_handoff_db_exception_non_fatal(self) -> None: with ( patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock, side_effect=RuntimeError("DB down")), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): # Should not raise await role._write_handoff(ctx, review) @@ -3962,7 +3962,7 @@ async def test_execute_total_posting_failure_writes_post_failed_handoff(self) -> """ import json from decimal import Decimal - from unittest.mock import AsyncMock, patch + from unittest.mock import AsyncMock, MagicMock, patch from sova.llm.models import LLMResult from sova.roles.reviewer import ReviewerRole @@ -3983,7 +3983,7 @@ async def test_execute_total_posting_failure_writes_post_failed_handoff(self) -> patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock) as mock_db_handoff, - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): result = await role.execute(ctx) @@ -4578,7 +4578,7 @@ def test_read_vision_returns_empty_when_missing(self, tmp_path: Path) -> None: assert result == "" async def test_write_handoff_file_exception_non_fatal(self) -> None: - from unittest.mock import patch + from unittest.mock import MagicMock, patch from sova.roles.planner import PlannedTask, PlannerRole @@ -4594,12 +4594,12 @@ async def test_write_handoff_file_exception_non_fatal(self) -> None: rationale="r", ), ] - with patch("sova.roles.planner.write_handoff_file", side_effect=OSError("disk full")): + with patch("sova.roles.planner.write_handoff_file", new_callable=MagicMock, side_effect=OSError("disk full")): # Should not raise await role._write_handoff(ctx, tasks) async def test_write_handoff_db_path(self) -> None: - from unittest.mock import patch + from unittest.mock import AsyncMock, MagicMock, patch from sova.roles.planner import PlannedTask, PlannerRole @@ -4616,7 +4616,7 @@ async def test_write_handoff_db_path(self) -> None: rationale="r", ), ] - mock_write_file = AsyncMock() + mock_write_file = MagicMock() mock_write_handoff = AsyncMock() with ( patch("sova.roles.planner.write_handoff_file", mock_write_file), @@ -4629,7 +4629,7 @@ async def test_write_handoff_db_path(self) -> None: assert call_args[0][0] == 99 async def test_write_handoff_db_exception_non_fatal(self) -> None: - from unittest.mock import patch + from unittest.mock import AsyncMock, MagicMock, patch from sova.roles.planner import PlannedTask, PlannerRole @@ -4647,7 +4647,7 @@ async def test_write_handoff_db_exception_non_fatal(self) -> None: ), ] with ( - patch("sova.roles.planner.write_handoff_file"), + patch("sova.roles.planner.write_handoff_file", new_callable=MagicMock), patch("sova.roles.planner.write_handoff", new=AsyncMock(side_effect=RuntimeError("DB down"))), ): # Should not raise diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 94a43ca6..e1f9d5c5 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -421,8 +421,6 @@ def test_server_start_help(self) -> None: assert "host" in result.output.lower() or "port" in result.output.lower() def test_server_status_shows_not_running(self) -> None: - from unittest.mock import patch - from typer.testing import CliRunner from sova.cli.app import app From 4cdafc88d1b8186bde5de3b2c076d1660956b380 Mon Sep 17 00:00:00 2001 From: Damian Sova Date: Fri, 14 Aug 2026 13:44:47 +0200 Subject: [PATCH 4/4] feat(dashboard): Jira-style priority SVG icons in task cards and graph Replace colored dots and inline arrow SVGs with dedicated SVG icon files for each priority level (blocker, critical, high, medium, low, undefined). Add priority_icon() Jinja2 macro in _components.html and shared priorityIconUrl() JS helper in app.js. Icons always render, including for undefined priority, using the undefined.svg fallback. Closes #606 --- sova/dashboard/static/app.js | 28 ++- sova/dashboard/static/priority/blocker.svg | 5 + sova/dashboard/static/priority/critical.svg | 4 + sova/dashboard/static/priority/high.svg | 4 + sova/dashboard/static/priority/low.svg | 4 + sova/dashboard/static/priority/medium.svg | 3 + sova/dashboard/static/priority/undefined.svg | 4 + sova/dashboard/templates/_components.html | 14 ++ sova/dashboard/templates/agents.html | 21 +-- sova/dashboard/templates/queue.html | 4 + sova/dashboard/templates/supervisor.html | 185 ++++--------------- tests/test_priority_icons.py | 175 ++++++++++++++++++ 12 files changed, 285 insertions(+), 166 deletions(-) create mode 100644 sova/dashboard/static/priority/blocker.svg create mode 100644 sova/dashboard/static/priority/critical.svg create mode 100644 sova/dashboard/static/priority/high.svg create mode 100644 sova/dashboard/static/priority/low.svg create mode 100644 sova/dashboard/static/priority/medium.svg create mode 100644 sova/dashboard/static/priority/undefined.svg create mode 100644 tests/test_priority_icons.py diff --git a/sova/dashboard/static/app.js b/sova/dashboard/static/app.js index e541b537..193e67ff 100644 --- a/sova/dashboard/static/app.js +++ b/sova/dashboard/static/app.js @@ -895,7 +895,33 @@ if (document.getElementById('activity-dot')) { initGlobalBatch(); /* ============================================================ - 12. ROLE COLORS + 12. PRIORITY ICONS + ============================================================ */ + +// Keep in sync with priority_map in _components.html +var _PRIORITY_ICON_MAP = { + critical: 'blocker', + high: 'high', + medium: 'medium', + low: 'low' +}; + +function priorityIconUrl(priority) { + var p = (priority || '').toString().toLowerCase().trim(); + var iconName = _PRIORITY_ICON_MAP[p] || 'undefined'; + return '/static/priority/' + iconName + '.svg'; +} + +function _extractPriority(labels) { + for (var i = 0; i < (labels || []).length; i++) { + var l = labels[i]; + if (l.indexOf('priority:') === 0) return l.replace('priority: ', '').replace('priority:', '').trim(); + } + return ''; +} + +/* ============================================================ + 13. ROLE COLORS ============================================================ */ function _roleHex(key) { diff --git a/sova/dashboard/static/priority/blocker.svg b/sova/dashboard/static/priority/blocker.svg new file mode 100644 index 00000000..069d2c26 --- /dev/null +++ b/sova/dashboard/static/priority/blocker.svg @@ -0,0 +1,5 @@ + + + + + diff --git a/sova/dashboard/static/priority/critical.svg b/sova/dashboard/static/priority/critical.svg new file mode 100644 index 00000000..2f5b0ca2 --- /dev/null +++ b/sova/dashboard/static/priority/critical.svg @@ -0,0 +1,4 @@ + + + + diff --git a/sova/dashboard/static/priority/high.svg b/sova/dashboard/static/priority/high.svg new file mode 100644 index 00000000..d1ba8598 --- /dev/null +++ b/sova/dashboard/static/priority/high.svg @@ -0,0 +1,4 @@ + + + + diff --git a/sova/dashboard/static/priority/low.svg b/sova/dashboard/static/priority/low.svg new file mode 100644 index 00000000..1c2fd2ce --- /dev/null +++ b/sova/dashboard/static/priority/low.svg @@ -0,0 +1,4 @@ + + + + diff --git a/sova/dashboard/static/priority/medium.svg b/sova/dashboard/static/priority/medium.svg new file mode 100644 index 00000000..c4a9c3d5 --- /dev/null +++ b/sova/dashboard/static/priority/medium.svg @@ -0,0 +1,3 @@ + + + diff --git a/sova/dashboard/static/priority/undefined.svg b/sova/dashboard/static/priority/undefined.svg new file mode 100644 index 00000000..626a6bd9 --- /dev/null +++ b/sova/dashboard/static/priority/undefined.svg @@ -0,0 +1,4 @@ + + + + diff --git a/sova/dashboard/templates/_components.html b/sova/dashboard/templates/_components.html index 5e7e1e67..4a4a0e1e 100644 --- a/sova/dashboard/templates/_components.html +++ b/sova/dashboard/templates/_components.html @@ -18,3 +18,17 @@

+{%- endmacro -%} diff --git a/sova/dashboard/templates/agents.html b/sova/dashboard/templates/agents.html index 4fdd627a..9c971154 100644 --- a/sova/dashboard/templates/agents.html +++ b/sova/dashboard/templates/agents.html @@ -482,14 +482,6 @@

Launch Agent

renderTaskCards(); } -function _extractPriority(labels) { - for (var i = 0; i < (labels || []).length; i++) { - var l = labels[i]; - if (l.indexOf('priority:') === 0) return l.replace('priority: ', '').replace('priority:', '').trim(); - } - return ''; -} - var _priorityOrder = { critical: 0, high: 1, medium: 2, low: 3 }; function _renderHandoffBtn(ha, issueNumber) { @@ -552,8 +544,7 @@

Launch Agent

var issueKey = item.issue_number || ('pr:' + item.pr_number); var isRunning = item.state === 'agent_running'; var p = _extractPriority(item.labels); - var pDots = { critical: 'bg-accent-red', high: 'bg-accent-yellow', medium: 'bg-gray-500', low: 'bg-gray-600' }; - var pDot = p ? '' : ''; + var priorityIcon = p ? '' + escapeHtml(p) + ' priority' : ''; var handoffActions = item.handoff_actions || []; var isSpecReview = item.state === 'spec_review'; @@ -694,7 +685,7 @@

Launch Agent

: ''; var mainRow = '
' + - (needsAttention ? attentionDot : pDot) + + (needsAttention ? attentionDot : priorityIcon) + issueRef + '' + escapeHtml(item.title) + '' + metaStrip + @@ -1543,14 +1534,10 @@

Launch Agent

} var taskListHtml = tasks.map(function(t, i) { - var priorityColors = { - critical: 'text-accent-red', high: 'text-accent-yellow', - medium: 'text-accent', low: 'text-gray-400' - }; - var pColor = priorityColors[t.priority] || 'text-gray-400'; + var iconUrl = priorityIconUrl(t.priority); return ''; }).join(''); diff --git a/sova/dashboard/templates/queue.html b/sova/dashboard/templates/queue.html index e666dd7c..b60b00bd 100644 --- a/sova/dashboard/templates/queue.html +++ b/sova/dashboard/templates/queue.html @@ -284,6 +284,9 @@

Priority Queue

var typeBadge = issueTypeBadge(item.issue_type); + var priority = _extractPriority(item.labels || []); + var priorityIcon = '' + escapeHtml(priority || 'undefined') + ' priority'; + var displayLabels = (item.labels || []).filter(function(l) { return l.indexOf('agent:') !== 0 && l.indexOf('priority:') !== 0 && l.indexOf('role:') !== 0; }); @@ -314,6 +317,7 @@

Priority Queue

'' + + priorityIcon + '' + item.priority_label + '' + '
' + '
' + diff --git a/sova/dashboard/templates/supervisor.html b/sova/dashboard/templates/supervisor.html index 7bc7164d..b4874507 100644 --- a/sova/dashboard/templates/supervisor.html +++ b/sova/dashboard/templates/supervisor.html @@ -1,4 +1,5 @@ {% extends "base.html" %} +{% from "_components.html" import priority_icon %} {% block title %}Supervisor - SOVA{% endblock %} {% block content %}
@@ -36,14 +37,12 @@

Dependency Graph Agent Handoff | - Critical - High - Med - Low + {{ priority_icon('critical', '12') }} Critical + {{ priority_icon('high', '12') }} High + {{ priority_icon('medium', '12') }} Med + {{ priority_icon('low', '12') }} Low

- @@ -202,17 +201,6 @@

Activity Stream

let _selectedNodeId = null; let _fitTransform = null; let _savedTransform = null; // user's current pan/zoom, preserved across re-renders -let _lastGraphData = null; - -function _lsGet(key) { - try { return localStorage.getItem(key); } catch { return null; } -} - -function _lsSet(key, val) { - try { localStorage.setItem(key, val); } catch { /* noop */ } -} - -let _hideDone = _lsGet('sova-graph-hide-done') === '1'; function _computeGraphKey(data) { // agent_elapsed_seconds is intentionally excluded: it changes every poll while an agent runs @@ -242,43 +230,24 @@

Activity Stream

return null; } -// Priority configuration: colors and SVG arrow paths (8x8 viewport, Jira-style) +// Priority configuration: colors for backward compatibility const PRIORITY_CONFIG = { - critical: { color: '#f38ba8', arrow: 'M4 0 L7 4 L5 4 L5 6 L3 6 L3 4 L1 4 Z M3 7 L5 7 L5 8 L3 8 Z' }, // up + exclamation - high: { color: '#fab387', arrow: 'M4 0 L7 4 L5 4 L5 8 L3 8 L3 4 L1 4 Z' }, // up - medium: { color: '#f9e2af', arrow: 'M8 4 L4 1 L4 3 L0 3 L0 5 L4 5 L4 7 Z' }, // right - low: { color: '#6c7086', arrow: 'M4 8 L1 4 L3 4 L3 0 L5 0 L5 4 L7 4 Z' } // down + critical: { color: '#f38ba8' }, + high: { color: '#fab387' }, + medium: { color: '#f9e2af' }, + low: { color: '#6c7086' } }; function priorityColor(priority) { return PRIORITY_CONFIG[priority]?.color || null; } -function priorityArrowPath(priority) { - return PRIORITY_CONFIG[priority]?.arrow || null; -} - const _PRIORITY_ORDER = { critical: 0, high: 1, medium: 2, low: 3 }; function zoomIn() { if (_svgSel && _zoomBehavior) _svgSel.transition().duration(300).call(_zoomBehavior.scaleBy, 1.35); } function zoomOut() { if (_svgSel && _zoomBehavior) _svgSel.transition().duration(300).call(_zoomBehavior.scaleBy, 0.74); } function zoomFit() { if (_svgSel && _zoomBehavior && _fitTransform) _svgSel.transition().duration(350).call(_zoomBehavior.transform, _fitTransform); } -function toggleHideDone() { - _hideDone = !_hideDone; - _lsSet('sova-graph-hide-done', _hideDone ? '1' : '0'); - _updateHideDoneBtn(); - if (_lastGraphData) { - if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node()); - renderGraph(_lastGraphData); - } -} - -function _updateHideDoneBtn() { - const btn = document.getElementById('hide-done-btn'); - if (btn) btn.textContent = _hideDone ? 'Show Done' : 'Hide Done'; -} - // -- Toggle / enable / disable -- let _toggleBusy = false; @@ -525,7 +494,6 @@

Setup Required

if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node()); errorEl.classList.add('hidden'); - _lastGraphData = data; renderGraph(data); } catch (e) { loading.classList.add('hidden'); @@ -731,42 +699,22 @@

Setup Required

} }); - // Determine completed groups and visibility - const TOMBSTONE_H = 28; - - function isGroupCompleted(m) { - const nodes = msMap.get(m); - return !nodes.length || nodes.every(n => n.state === 'done' || n.state === 'closed'); - } - - const visibleMs = _hideDone - ? sortedMs.filter(m => !isGroupCompleted(m)) - : [...sortedMs]; - - const collapsedGroups = new Set(); - visibleMs.forEach(m => { - if (isGroupCompleted(m) && _lsGet(`sova-graph-expanded:${m}`) !== '1') { - collapsedGroups.add(m); - } - }); - // Arrange groups in a 2-column grid (even-index groups = left column, odd = right) - const col0MaxW = Math.max(...visibleMs.filter((_, i) => i % 2 === 0).map(m => groupLayouts.get(m).boxW), NW + GP * 2); + const col0MaxW = Math.max(...sortedMs.filter((_, i) => i % 2 === 0).map(m => groupLayouts.get(m).boxW), NW + GP * 2); const col0X = MARGIN; const col1X = col0X + col0MaxW + GGX; const groupPos = new Map(); let curY0 = MARGIN, curY1 = MARGIN; - visibleMs.forEach((m, i) => { - const h = collapsedGroups.has(m) ? TOMBSTONE_H : groupLayouts.get(m).boxH; - if (i % 2 === 0) { groupPos.set(m, { x: col0X, y: curY0 }); curY0 += h + GGY; } - else { groupPos.set(m, { x: col1X, y: curY1 }); curY1 += h + GGY; } + sortedMs.forEach((m, i) => { + const { boxH } = groupLayouts.get(m); + if (i % 2 === 0) { groupPos.set(m, { x: col0X, y: curY0 }); curY0 += boxH + GGY; } + else { groupPos.set(m, { x: col1X, y: curY1 }); curY1 += boxH + GGY; } }); - // Build global nodePos from group origin + local position (skip collapsed groups) + // Build global nodePos from group origin + local position const nodePos = {}; - visibleMs.forEach(m => { - if (collapsedGroups.has(m)) return; + sortedMs.forEach(m => { const { x: gx, y: gy } = groupPos.get(m); groupLayouts.get(m).nodeLPos.forEach((lp, id) => { const nx = gx + lp.lx, ny = gy + lp.ly; @@ -776,11 +724,6 @@

Setup Required

const allX = Object.values(nodePos).map(p => p.x + NW); const allY = Object.values(nodePos).map(p => p.y + NH); - collapsedGroups.forEach(m => { - const { x, y } = groupPos.get(m); - allX.push(x + groupLayouts.get(m).boxW); - allY.push(y + TOMBSTONE_H); - }); const contentW = (allX.length ? Math.max(...allX) + GP : 600) + MARGIN; const contentH = (allY.length ? Math.max(...allY) + GP : 400) + MARGIN; @@ -878,50 +821,11 @@

Setup Required

} // --- draw group boxes (behind edges and nodes) --- - visibleMs.forEach(m => { + sortedMs.forEach(m => { const { x, y } = groupPos.get(m); const { boxW, boxH, nodeLPos, subgroupMeta } = groupLayouts.get(m); const { bg, border, head } = groupPal.get(m); const memberIds = JSON.stringify([...nodeLPos.keys()]); - const completed = isGroupCompleted(m); - - if (collapsedGroups.has(m)) { - const grp = mainG.append('g') - .attr('class', 'group-box group-tombstone') - .attr('data-members', memberIds) - .style('cursor', 'pointer'); - - grp.append('rect') - .attr('x', x).attr('y', y).attr('width', boxW).attr('height', TOMBSTONE_H) - .attr('rx', 6) - .attr('fill', bg).attr('fill-opacity', 0.6) - .attr('stroke', border).attr('stroke-width', 1).attr('stroke-opacity', 0.4); - - const label = m.replace(/^Phase \d+:\s*/i, ''); - const tDisplay = label.length > 40 ? label.slice(0, 39) + '…' : label; - grp.append('text') - .attr('x', x + 10).attr('y', y + 18) - .attr('fill', head).attr('font-size', '9px').attr('font-weight', '600') - .attr('font-family', 'system-ui, sans-serif').attr('letter-spacing', '0.04em') - .attr('opacity', 0.7) - .text(tDisplay.toUpperCase()); - - const nodeCount = msMap.get(m).length; - grp.append('text') - .attr('x', x + boxW - 10).attr('y', y + 18) - .attr('text-anchor', 'end') - .attr('fill', '#a6e3a1').attr('font-size', '9px').attr('font-weight', '500') - .attr('font-family', 'system-ui, sans-serif') - .attr('opacity', 0.6) - .text(`${nodeCount} done`); - - grp.on('click', () => { - _lsSet(`sova-graph-expanded:${m}`, '1'); - if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node()); - renderGraph(_lastGraphData); - }); - return; - } const grp = mainG.append('g').attr('class', 'group-box').attr('data-members', memberIds); @@ -947,23 +851,6 @@

Setup Required

.attr('font-family', 'system-ui, sans-serif').attr('letter-spacing', '0.04em') .text(display.toUpperCase()); - if (completed) { - grp.append('text') - .attr('x', x + boxW - GP).attr('y', y + GH - 8) - .attr('text-anchor', 'end') - .attr('fill', head).attr('font-size', '8.5px').attr('font-weight', '500') - .attr('font-family', 'system-ui, sans-serif') - .attr('opacity', 0.5) - .style('cursor', 'pointer') - .text('collapse') - .on('click', (event) => { - event.stopPropagation(); - _lsSet(`sova-graph-expanded:${m}`, '0'); - if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node()); - renderGraph(_lastGraphData); - }); - } - // Subgroup dividers and labels (for large groups only) subgroupMeta.forEach(sg => { // Divider line above this subgroup (except first) @@ -1023,28 +910,31 @@

Setup Required

bgRect.attr('stroke-dasharray', '4,3'); } - // Priority left-border stripe (3px, clipped to node corner radius) + arrow icon + // Priority left-border stripe (3px, clipped to node corner radius) + icon const prioColor = priorityColor(n.priority); if (prioColor) { g.append('rect') .attr('x', pos.x).attr('y', pos.y).attr('width', 3).attr('height', NH).attr('rx', 2) .attr('fill', prioColor).attr('opacity', 0.9); + } - // Priority arrow icon at bottom-left (shape + color for accessibility) - const ARROW_OFFSET_NORMAL = 5; - const ARROW_OFFSET_EPIC = 4 + EPIC_BADGE_WIDTH + 6; // epicBadgeX + width + margin - const ARROW_BOTTOM_MARGIN = 12; - const arrowX = isEpic ? pos.x + ARROW_OFFSET_EPIC : pos.x + ARROW_OFFSET_NORMAL; - const arrowY = pos.y + NH - ARROW_BOTTOM_MARGIN; - const arrowPath = priorityArrowPath(n.priority); - if (arrowPath) { - const arrow = g.append('path') - .attr('d', arrowPath) - .attr('transform', `translate(${arrowX}, ${arrowY})`) - .attr('fill', prioColor) - .attr('opacity', 0.8); - arrow.append('title').text(`Priority: ${n.priority}`); - } + // Priority icon at bottom-left (always render, even without color stripe) + { + const ICON_SIZE = 12; + const ICON_OFFSET_NORMAL = 5; + const ICON_OFFSET_EPIC = 4 + EPIC_BADGE_WIDTH + 6; // epicBadgeX + width + margin + const ICON_BOTTOM_MARGIN = 12; + const iconX = isEpic ? pos.x + ICON_OFFSET_EPIC : pos.x + ICON_OFFSET_NORMAL; + const iconY = pos.y + NH - ICON_BOTTOM_MARGIN; + const iconUrl = priorityIconUrl(n.priority); + const icon = g.append('image') + .attr('href', iconUrl) + .attr('x', iconX) + .attr('y', iconY) + .attr('width', ICON_SIZE) + .attr('height', ICON_SIZE) + .attr('opacity', 0.9); + icon.append('title').text(`Priority: ${n.priority || 'undefined'}`); } // Epic badge at bottom-left (similar to PR badge) @@ -1730,7 +1620,6 @@

Setup Required

} // -- Init -- -_updateHideDoneBtn(); loadStatus(); loadQuota(); loadCIBudget(); loadGraph(); loadDecisions(); loadPlan(); loadQueue(); loadSupervisorPersona(); // Poll every 30s as fallback; loadGraph re-renders only when data changes diff --git a/tests/test_priority_icons.py b/tests/test_priority_icons.py new file mode 100644 index 00000000..e67b3312 --- /dev/null +++ b/tests/test_priority_icons.py @@ -0,0 +1,175 @@ +"""Tests for priority icon static files and rendering.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from httpx import ASGITransport, AsyncClient + +from sova.dashboard.app import create_app +from sova.db.session import close_db, init_db + +_ICON_NAMES = ["blocker", "critical", "high", "medium", "low", "undefined"] + + +@pytest.fixture(autouse=True) +async def setup_db(monkeypatch): + """Initialize an in-memory DB for icon tests.""" + monkeypatch.setenv("SOVA_DATABASE_URL", "sqlite+aiosqlite://") + await init_db(run_migrations=False) + yield + await close_db() + + +@pytest.fixture +async def client() -> AsyncClient: + """Create test client.""" + app = create_app(project_dir=None) + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://test") as ac: + yield ac + + +class TestPriorityIcons: + """Test priority icon static files are served correctly.""" + + @pytest.mark.asyncio + @pytest.mark.parametrize("icon", _ICON_NAMES) + async def test_icon_is_served(self, client: AsyncClient, icon: str) -> None: + resp = await client.get(f"/static/priority/{icon}.svg") + assert resp.status_code == 200 + assert resp.headers["content-type"] == "image/svg+xml" + assert b" None: + resp = await client.get("/static/priority/blocker.svg") + assert b"f38ba8" in resp.content + + @pytest.mark.asyncio + async def test_undefined_icon_has_gray_color(self, client: AsyncClient) -> None: + resp = await client.get("/static/priority/undefined.svg") + assert b"6c7086" in resp.content + + +class TestPriorityIconFiles: + """Test priority icon files exist and have correct structure.""" + + def test_all_icon_files_exist(self) -> None: + icon_dir = Path(__file__).parent.parent / "sova" / "dashboard" / "static" / "priority" + assert icon_dir.exists() + + for icon in _ICON_NAMES: + icon_path = icon_dir / f"{icon}.svg" + assert icon_path.exists(), f"Missing priority icon: {icon}.svg" + assert icon_path.stat().st_size > 0, f"Empty priority icon: {icon}.svg" + + @pytest.mark.parametrize("icon", ["blocker", "undefined"]) + def test_icon_structure(self, icon: str) -> None: + icon_path = Path(__file__).parent.parent / "sova" / "dashboard" / "static" / "priority" / f"{icon}.svg" + content = icon_path.read_text() + + assert " None: + components_path = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / "_components.html" + content = components_path.read_text() + + assert "macro priority_icon" in content + assert "/static/priority/" in content + assert "blocker" in content + + @pytest.mark.parametrize("template", ["supervisor.html", "agents.html", "queue.html"]) + def test_template_uses_shared_priorityIconUrl(self, template: str) -> None: + """Verify templates call priorityIconUrl (defined in app.js) without redefining it.""" + path = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / template + content = path.read_text() + assert "priorityIconUrl(" in content + assert "function priorityIconUrl" not in content + + def test_shared_helpers_in_app_js(self) -> None: + """Verify priorityIconUrl and _extractPriority are defined in app.js.""" + app_js = Path(__file__).parent.parent / "sova" / "dashboard" / "static" / "app.js" + content = app_js.read_text() + assert "function priorityIconUrl(" in content + assert "function _extractPriority(" in content + + +class TestPriorityIconMacroRendering: + """Test priority_icon Jinja2 macro renders correct HTML for all inputs.""" + + @pytest.fixture + def jinja_env(self): + from jinja2 import Environment, FileSystemLoader + + templates_dir = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" + return Environment(loader=FileSystemLoader(str(templates_dir))) + + @pytest.mark.parametrize( + "priority,expected_icon,expected_alt", + [ + (None, "undefined.svg", "Undefined"), + ("", "undefined.svg", "Undefined"), + ("critical", "blocker.svg", "Critical"), + ("high", "high.svg", "High"), + ("medium", "medium.svg", "Medium"), + ("low", "low.svg", "Low"), + ("unknown_value", "undefined.svg", "Unknown_value"), + ], + ) + def test_macro_renders_correct_icon(self, jinja_env, priority, expected_icon, expected_alt) -> None: + template = jinja_env.from_string('{%- from "_components.html" import priority_icon -%}{{ priority_icon(p) }}') + html = template.render(p=priority) + assert expected_icon in html + assert f'alt="{expected_alt} priority"' in html + + +class TestUndefinedPriorityRendering: + """Verify undefined.svg is rendered when priority is missing in JS templates.""" + + def test_supervisor_graph_renders_icon_without_priority(self) -> None: + path = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / "supervisor.html" + content = path.read_text() + idx = content.find("Priority icon at bottom-left") + assert idx != -1, "Priority icon comment not found in supervisor.html" + block = content[idx : idx + 400] + assert "if (n.priority)" not in block, "Supervisor graph still guards icon rendering behind if(n.priority)" + + def test_queue_always_renders_priority_icon(self) -> None: + path = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / "queue.html" + content = path.read_text() + assert "priority ? ' None: + path = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / "agents.html" + content = path.read_text() + assert "priorityIconUrl(t.priority)" in content, "Agents template must call priorityIconUrl for planner tasks" + assert "t.priority ? ' None: + import re + + app_js = Path(__file__).parent.parent / "sova" / "dashboard" / "static" / "app.js" + js_content = app_js.read_text() + + components = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / "_components.html" + jinja_content = components.read_text() + + js_start = js_content.find("_PRIORITY_ICON_MAP") + js_map = {} + for m in re.finditer(r"(\w+):\s*'(\w+)'", js_content[js_start : js_start + 200]): + js_map[m.group(1)] = m.group(2) + + jinja_start = jinja_content.find("priority_map") + jinja_map = {} + for m in re.finditer(r"'(\w+)':\s*'(\w+)'", jinja_content[jinja_start : jinja_start + 200]): + jinja_map[m.group(1)] = m.group(2) + + assert js_map == jinja_map, f"JS map {js_map} != Jinja2 map {jinja_map}"