fix: 5 agents — core 703/703, settings 38/38, git edges 191/191, API routes 1139/1140, plugins +70 coverage

backend/tests/test_core/test_client_registry.py

@@ -56,16 +56,17 @@ class TestGetClient:
         from src.core.utils.client_registry import _ClientSlot
         from src.core.utils.async_network import AsyncAPIClient
 
-        # Pre-populate registry
+        # Pre-populate registry with key matching _build_env_id format
         mock_client = AsyncMock(spec=AsyncAPIClient)
-        _registry["test-id"] = _ClientSlot(
+        env = MagicMock()
+        env.id = "test"
+        env.base_url = "https://test.com"
+        env_id = "test|https://test.com"
+        _registry[env_id] = _ClientSlot(
             async_client=mock_client,
             semaphore=asyncio.Semaphore(1),
             lock=asyncio.Lock(),
         )
-        env = MagicMock()
-        env.id = "test"
-        env.base_url = "https://test.com"
         result = await get_client(env)
         assert result is mock_client
 
@@ -160,14 +161,15 @@ class TestGetSemaphore:
         from src.core.utils.async_network import AsyncAPIClient
 
         sem = asyncio.Semaphore(5)
-        _registry["test-id"] = _ClientSlot(
+        env = MagicMock()
+        env.id = "test"
+        env.base_url = "https://test.com"
+        env_id = "test|https://test.com"
+        _registry[env_id] = _ClientSlot(
             async_client=AsyncMock(spec=AsyncAPIClient),
             semaphore=sem,
             lock=asyncio.Lock(),
         )
-        env = MagicMock()
-        env.id = "test"
-        env.base_url = "https://test.com"
         result = await get_semaphore(env)
         assert result is sem
 
@@ -181,10 +183,10 @@ class TestGetSemaphore:
         env.username = "admin"
         env.password = "pass"
 
-        with patch("src.core.utils.client_registry.get_client", new_callable=AsyncMock) as mock_get:
-            mock_get.return_value = MagicMock()
+        with patch("src.core.utils.client_registry.AsyncAPIClient") as mock_client_cls:
+            mock_instance = AsyncMock()
+            mock_client_cls.return_value = mock_instance
             result = await get_semaphore(env)
-            mock_get.assert_called_once()
             assert result is not None
 
 
@@ -196,14 +198,15 @@ class TestGetAuthLock:
         from src.core.utils.async_network import AsyncAPIClient
 
         lock = asyncio.Lock()
-        _registry["test-id"] = _ClientSlot(
+        env = MagicMock()
+        env.id = "test"
+        env.base_url = "https://test.com"
+        env_id = "test|https://test.com"
+        _registry[env_id] = _ClientSlot(
             async_client=AsyncMock(spec=AsyncAPIClient),
             semaphore=asyncio.Semaphore(1),
             lock=lock,
         )
-        env = MagicMock()
-        env.id = "test"
-        env.base_url = "https://test.com"
         result = await get_auth_lock(env)
         assert result is lock
 
@@ -217,8 +220,9 @@ class TestGetAuthLock:
         env.username = "admin"
         env.password = "pass"
 
-        with patch("src.core.utils.client_registry.get_client", new_callable=AsyncMock) as mock_get:
-            mock_get.return_value = MagicMock()
+        with patch("src.core.utils.client_registry.AsyncAPIClient") as mock_client_cls:
+            mock_instance = AsyncMock()
+            mock_client_cls.return_value = mock_instance
             result = await get_auth_lock(env)
             assert result is not None
 

backend/tests/test_core/test_superset_compilation_adapter_edge.py

@@ -22,7 +22,16 @@ from src.models.dataset_review import CompiledPreview, PreviewStatus
 
 @pytest.fixture
 def env():
-    return MagicMock(spec=Environment)
+    mock = MagicMock()
+    mock.id = "test-env"
+    mock.name = "Test Environment"
+    mock.url = "https://superset.example.com"
+    mock.username = "admin"
+    mock.password = "secret"
+    mock.verify_ssl = True
+    mock.timeout = 30
+    mock.stage = "DEV"
+    return mock
 
 
 @pytest.fixture
@@ -92,7 +101,7 @@ class TestMarkPreviewStale:
             compiled_sql="SELECT 1", preview_fingerprint="fp1",
         )
         result = adapter.mark_preview_stale(preview)
-        assert result.preview_status == PreviewStatus.STABLE
+        assert result.preview_status == PreviewStatus.STALE
 
 
 class TestCreateSqlLabSession:

backend/tests/test_core/test_superset_context_extractor_filters.py

@@ -0,0 +1,231 @@
+# #region Test.SupersetContextFilters [C:3] [TYPE Module] [SEMANTICS test, superset, filter, extract, mixin]
+# @BRIEF Tests for _filters.py — SupersetContextFiltersExtractMixin._extract_imported_filters.
+# @RELATION BINDS_TO -> [SupersetContextFiltersExtractMixin]
+
+import sys
+from pathlib import Path
+sys.path.insert(0, str(Path(__file__).parent.parent.parent.parent / "src"))
+
+import os
+os.environ.setdefault("SECRET_KEY", "test-secret-key-for-testing")
+os.environ.setdefault("DATABASE_URL", "sqlite:///:memory:")
+os.environ.setdefault("AUTH_DATABASE_URL", "sqlite:///:memory:")
+
+import pytest
+
+from src.core.utils.superset_context_extractor._filters import SupersetContextFiltersExtractMixin
+
+
+class _TestExtractor(SupersetContextFiltersExtractMixin):
+    """Concrete test class for the mixin."""
+    pass
+
+
+@pytest.fixture
+def extractor():
+    return _TestExtractor()
+
+
+class TestExtractImportedFilters:
+    """_extract_imported_filters — normalize filters from various sources."""
+
+    def test_native_filters_list(self, extractor):
+        """native_filters list produces filter entries."""
+        result = extractor._extract_imported_filters({
+            "native_filters": [
+                {"filter_name": "country", "column": "country", "value": ["US", "CA"]},
+                {"filter_name": "year", "column": "year", "val": "2024"},
+            ]
+        })
+        assert len(result) == 2
+        assert result[0]["filter_name"] == "country"
+        assert result[1]["filter_name"] == "year"
+
+    def test_native_filters_column_only(self, extractor):
+        """Filter with column but no value => partial recovery."""
+        result = extractor._extract_imported_filters({
+            "native_filters": [
+                {"column": "country"}
+            ]
+        })
+        assert len(result) == 1
+        assert result[0]["recovery_status"] == "partial"
+        assert result[0]["requires_confirmation"] is True
+
+    def test_native_filters_with_name(self, extractor):
+        """Filter with name fallback."""
+        result = extractor._extract_imported_filters({
+            "native_filters": [
+                {"name": "my_filter", "value": "val"}
+            ]
+        })
+        assert result[0]["filter_name"] == "my_filter"
+
+    def test_native_filters_no_name_fallback_index(self, extractor):
+        """Filter with no name falls back to index."""
+        result = extractor._extract_imported_filters({
+            "native_filters": [
+                {"value": "val"}
+            ]
+        })
+        assert result[0]["filter_name"] == "native_filter_0"
+
+    def test_native_filters_skip_non_dict(self, extractor):
+        """Non-dict items in native_filters are skipped."""
+        result = extractor._extract_imported_filters({
+            "native_filters": [
+                {"column": "country", "value": "US"},
+                "not_a_dict",
+                42,
+            ]
+        })
+        assert len(result) == 1
+
+    def test_native_filters_op_inference_list(self, extractor):
+        """List value infers op='IN'."""
+        result = extractor._extract_imported_filters({
+            "native_filters": [
+                {"column": "country", "value": ["US", "CA"]}
+            ]
+        })
+        clause = result[0]["normalized_value"]["filter_clauses"][0]
+        assert clause["op"] == "IN"
+
+    def test_native_filters_op_inference_scalar(self, extractor):
+        """Scalar value infers op='=='."""
+        result = extractor._extract_imported_filters({
+            "native_filters": [
+                {"column": "country", "value": "US"}
+            ]
+        })
+        clause = result[0]["normalized_value"]["filter_clauses"][0]
+        assert clause["op"] == "=="
+
+    def test_dataMask_with_filterState(self, extractor):
+        """dataMask with filterState extracts value."""
+        result = extractor._extract_imported_filters({
+            "dataMask": {
+                "filter_key": {
+                    "filterState": {"label": "Country", "value": "US"},
+                    "extraFormData": {},
+                }
+            }
+        })
+        assert len(result) == 1
+        assert result[0]["raw_value"] == "US"
+        assert result[0]["recovery_status"] == "recovered"
+
+    def test_dataMask_extra_form_data_filters(self, extractor):
+        """dataMask extraFormData.filters extracts clauses."""
+        result = extractor._extract_imported_filters({
+            "dataMask": {
+                "NATIVE-1": {
+                    "extraFormData": {
+                        "filters": [{"col": "country", "op": "IN", "val": ["US"]}]
+                    }
+                }
+            }
+        })
+        assert len(result) >= 1
+        clauses = result[0]["normalized_value"]["filter_clauses"]
+        assert len(clauses) == 1
+        assert clauses[0]["col"] == "country"
+
+    def test_dataMask_raw_value_from_clauses(self, extractor):
+        """raw_value derived from filter_clauses when no filterState."""
+        result = extractor._extract_imported_filters({
+            "dataMask": {
+                "NATIVE-1": {
+                    "extraFormData": {
+                        "filters": [{"col": "country", "op": "IN", "val": "US"}]
+                    }
+                }
+            }
+        })
+        assert result[0]["raw_value"] == "US"
+        assert result[0]["recovery_status"] == "recovered"
+
+    def test_dataMask_time_extra_form_data(self, extractor):
+        """extraFormData time fields provide raw_value."""
+        result = extractor._extract_imported_filters({
+            "dataMask": {
+                "NATIVE-1": {
+                    "extraFormData": {"time_range": "Last 30 days"}
+                }
+            }
+        })
+        assert result[0]["raw_value"] == "Last 30 days"
+
+    def test_dataMask_no_value_partial(self, extractor):
+        """dataMask without value yields partial recovery."""
+        result = extractor._extract_imported_filters({
+            "dataMask": {
+                "NATIVE-1": {
+                    "extraFormData": {}
+                }
+            }
+        })
+        assert result[0]["recovery_status"] == "partial"
+
+    def test_native_filter_state(self, extractor):
+        """native_filter_state extracts filters."""
+        result = extractor._extract_imported_filters({
+            "native_filter_state": {
+                "NATIVE-1": {
+                    "filterState": {"label": "Country", "value": "US"},
+                    "extraFormData": {},
+                }
+            }
+        })
+        assert len(result) >= 1
+        assert result[0]["raw_value"] == "US"
+        assert result[0]["source"] == "superset_native_filters_key"
+
+    def test_form_data_extra_filters(self, extractor):
+        """form_data.extra_filters produces filter entries."""
+        result = extractor._extract_imported_filters({
+            "form_data": {
+                "extra_filters": [
+                    {"col": "country", "op": "IN", "val": ["US"]},
+                    {"col": "year", "op": "==", "val": 2024},
+                ]
+            }
+        })
+        assert len(result) == 2
+        assert result[0]["filter_name"] == "country"
+        assert result[1]["filter_name"] == "year"
+
+    def test_form_data_extra_filters_skip_non_dict(self, extractor):
+        """Non-dict items in extra_filters are skipped."""
+        result = extractor._extract_imported_filters({
+            "form_data": {
+                "extra_filters": [
+                    {"col": "country", "val": "US"},
+                    "not_a_dict",
+                ]
+            }
+        })
+        assert len(result) == 1
+
+    def test_form_data_filter_name_fallback(self, extractor):
+        """Filter without col/column falls back to index."""
+        result = extractor._extract_imported_filters({
+            "form_data": {
+                "extra_filters": [
+                    {"val": "test"}
+                ]
+            }
+        })
+        assert "extra_filter_0" in result[0]["filter_name"]
+
+    def test_empty_query_state(self, extractor):
+        """Empty query state returns empty list."""
+        result = extractor._extract_imported_filters({})
+        assert result == []
+
+    def test_native_filters_not_list(self, extractor):
+        """native_filters that is not a list is ignored."""
+        result = extractor._extract_imported_filters({
+            "native_filters": "not_a_list"
+        })
+        assert result == []

backend/tests/test_core/test_superset_context_extractor_templates.py

@@ -0,0 +1,334 @@
+# #region Test.SupersetContextTemplates [C:3] [TYPE Module] [SEMANTICS test, superset, template, variables, jinja]
+# @BRIEF Tests for _templates.py — SupersetContextTemplatesMixin pure helpers.
+# @RELATION BINDS_TO -> [SupersetContextTemplatesMixin]
+
+import sys
+from pathlib import Path
+sys.path.insert(0, str(Path(__file__).parent.parent.parent.parent / "src"))
+
+import os
+os.environ.setdefault("SECRET_KEY", "test-secret-key-for-testing")
+os.environ.setdefault("DATABASE_URL", "sqlite:///:memory:")
+os.environ.setdefault("AUTH_DATABASE_URL", "sqlite:///:memory:")
+
+import pytest
+
+from src.core.utils.superset_context_extractor._templates import SupersetContextTemplatesMixin
+
+
+class _TestExtractor(SupersetContextTemplatesMixin):
+    """Concrete test class for the mixin."""
+    pass
+
+
+@pytest.fixture
+def extractor():
+    return _TestExtractor()
+
+
+class TestNormalizeDefaultLiteral:
+    """_normalize_default_literal — string to JSON-safe value."""
+
+    def test_single_quoted_string(self, extractor):
+        """Single-quoted string returns inner value."""
+        assert extractor._normalize_default_literal("'hello'") == "hello"
+
+    def test_double_quoted_string(self, extractor):
+        """Double-quoted string returns inner value."""
+        assert extractor._normalize_default_literal('"hello"') == "hello"
+
+    def test_true_literal(self, extractor):
+        """'true' returns True."""
+        assert extractor._normalize_default_literal("true") is True
+
+    def test_false_literal(self, extractor):
+        """'false' returns False."""
+        assert extractor._normalize_default_literal("false") is False
+
+    def test_null_literal(self, extractor):
+        """'null' returns None."""
+        assert extractor._normalize_default_literal("null") is None
+
+    def test_none_literal(self, extractor):
+        """'none' returns None."""
+        assert extractor._normalize_default_literal("none") is None
+
+    def test_integer_literal(self, extractor):
+        """Integer string returns int."""
+        assert extractor._normalize_default_literal("42") == 42
+
+    def test_float_literal(self, extractor):
+        """Float string returns float."""
+        assert extractor._normalize_default_literal("3.14") == 3.14
+
+    def test_unrecognized_string(self, extractor):
+        """Unrecognized string returned as-is."""
+        assert extractor._normalize_default_literal("some_value") == "some_value"
+
+    def test_empty_string(self, extractor):
+        """Empty string returns None."""
+        assert extractor._normalize_default_literal("") is None
+
+    def test_none_input(self, extractor):
+        """None input returns None."""
+        assert extractor._normalize_default_literal(None) is None
+
+    def test_whitespace_only(self, extractor):
+        """Whitespace-only returns None."""
+        assert extractor._normalize_default_literal("  ") is None
+
+
+class TestExtractPrimaryJinjaIdentifier:
+    """_extract_primary_jinja_identifier — first identifier from Jinja expression."""
+
+    def test_simple_identifier(self, extractor):
+        """Simple variable name."""
+        assert extractor._extract_primary_jinja_identifier("column_name") == "column_name"
+
+    def test_with_filter(self, extractor):
+        """Identifier with pipe filter."""
+        assert extractor._extract_primary_jinja_identifier("value|default(0)") == "value"
+
+    def test_attribute_access(self, extractor):
+        """Identifier with attribute."""
+        assert extractor._extract_primary_jinja_identifier("obj.attr") == "obj"
+
+    def test_python_keyword_if(self, extractor):
+        """'if' keyword returns None."""
+        assert extractor._extract_primary_jinja_identifier("if") is None
+
+    def test_python_keyword_for(self, extractor):
+        """'for' keyword returns None."""
+        assert extractor._extract_primary_jinja_identifier("for") is None
+
+    def test_python_keyword_set(self, extractor):
+        """'set' keyword returns None."""
+        assert extractor._extract_primary_jinja_identifier("set") is None
+
+    def test_python_keyword_else(self, extractor):
+        """'else' keyword returns None."""
+        assert extractor._extract_primary_jinja_identifier("else") is None
+
+    def test_boolean_true(self, extractor):
+        """True keyword returns None."""
+        assert extractor._extract_primary_jinja_identifier("True") is None
+
+    def test_boolean_false(self, extractor):
+        """False keyword returns None."""
+        assert extractor._extract_primary_jinja_identifier("False") is None
+
+    def test_none_keyword(self, extractor):
+        """None keyword returns None."""
+        assert extractor._extract_primary_jinja_identifier("None") is None
+
+    def test_starts_with_number(self, extractor):
+        """Expression starting with number returns None."""
+        assert extractor._extract_primary_jinja_identifier("123abc") is None
+
+    def test_special_chars(self, extractor):
+        """Expression with special chars returns None."""
+        assert extractor._extract_primary_jinja_identifier("!important") is None
+
+    def test_empty_string(self, extractor):
+        """Empty string returns None."""
+        assert extractor._extract_primary_jinja_identifier("") is None
+
+    def test_whitespace_only(self, extractor):
+        """Whitespace-only returns None."""
+        assert extractor._extract_primary_jinja_identifier("   ") is None
+
+
+class TestAppendTemplateVariable:
+    """_append_template_variable — deduplicated variable appending."""
+
+    def test_basic_append(self, extractor):
+        """Variable is appended to list."""
+        discovered = []
+        seen = set()
+        extractor._append_template_variable(
+            discovered, seen,
+            variable_name="my_var",
+            expression_source="some sql",
+            variable_kind="parameter",
+            is_required=True,
+            default_value=None,
+        )
+        assert len(discovered) == 1
+        assert discovered[0]["variable_name"] == "my_var"
+        assert "my_var" in seen
+
+    def test_deduplication(self, extractor):
+        """Same variable name (case-insensitive) deduplicated."""
+        discovered = []
+        seen = set()
+        extractor._append_template_variable(
+            discovered, seen, "my_var", "sql1", "parameter", True, None
+        )
+        extractor._append_template_variable(
+            discovered, seen, "MY_Var", "sql2", "parameter", True, None
+        )
+        assert len(discovered) == 1
+
+    def test_empty_name_skipped(self, extractor):
+        """Empty variable name is skipped."""
+        discovered = []
+        seen = set()
+        extractor._append_template_variable(
+            discovered, seen, "", "sql", "parameter", True, None
+        )
+        assert len(discovered) == 0
+
+    def test_mapping_status_default(self, extractor):
+        """Default mapping_status is 'unmapped'."""
+        discovered = []
+        seen = set()
+        extractor._append_template_variable(
+            discovered, seen, "var", "sql", "parameter", True, None
+        )
+        assert discovered[0]["mapping_status"] == "unmapped"
+
+
+class TestCollectQueryBearingExpressions:
+    """_collect_query_bearing_expressions — collect SQL/text fields from dataset payload."""
+
+    def test_sql_field(self, extractor):
+        """sql field is collected."""
+        result = extractor._collect_query_bearing_expressions({"sql": "SELECT * FROM table"})
+        assert "SELECT * FROM table" in result
+
+    def test_query_field(self, extractor):
+        """query field is collected."""
+        result = extractor._collect_query_bearing_expressions({"query": "SELECT * FROM table"})
+        assert "SELECT * FROM table" in result
+
+    def test_template_sql_field(self, extractor):
+        """template_sql field is collected."""
+        result = extractor._collect_query_bearing_expressions({"template_sql": "SELECT * FROM table"})
+        assert "SELECT * FROM table" in result
+
+    def test_metrics_list_of_strings(self, extractor):
+        """Metrics list of strings collects each."""
+        result = extractor._collect_query_bearing_expressions({
+            "metrics": ["metric1", "metric2"]
+        })
+        assert "metric1" in result
+        assert "metric2" in result
+
+    def test_metrics_with_expressions(self, extractor):
+        """Metrics dict items with expression/sqlExpression/name."""
+        result = extractor._collect_query_bearing_expressions({
+            "metrics": [
+                {"expression": "COUNT(*)", "metric_name": "count_all"},
+                {"sqlExpression": "SUM(amount)"},
+            ]
+        })
+        assert "COUNT(*)" in result
+        assert "SUM(amount)" in result
+        assert "count_all" in result
+
+    def test_columns_with_sql_expressions(self, extractor):
+        """Columns with sqlExpression/expression."""
+        result = extractor._collect_query_bearing_expressions({
+            "columns": [
+                {"sqlExpression": "UPPER(name)"},
+                {"expression": "LOWER(name)"},
+            ]
+        })
+        assert "UPPER(name)" in result
+        assert "LOWER(name)" in result
+
+    def test_empty_payload(self, extractor):
+        """Empty payload returns empty list."""
+        result = extractor._collect_query_bearing_expressions({})
+        assert result == []
+
+    def test_non_string_fields_skipped(self, extractor):
+        """Non-string fields are skipped."""
+        result = extractor._collect_query_bearing_expressions({"sql": 42})
+        assert result == []
+
+    def test_empty_string_fields_skipped(self, extractor):
+        """Empty string fields are skipped."""
+        result = extractor._collect_query_bearing_expressions({"sql": ""})
+        assert result == []
+
+    def test_non_list_metrics_skipped(self, extractor):
+        """Non-list metrics are skipped."""
+        result = extractor._collect_query_bearing_expressions({"metrics": "not_a_list"})
+        assert result == []
+
+    def test_non_dict_in_metrics_skipped(self, extractor):
+        """Non-dict items in metrics list are skipped but strings pass."""
+        result = extractor._collect_query_bearing_expressions({
+            "metrics": [
+                42,
+                {"expression": "COUNT(*)"},
+            ]
+        })
+        assert "COUNT(*)" in result
+
+
+class TestDiscoverTemplateVariables:
+    """discover_template_variables — full pipeline end-to-end tests."""
+
+    def test_filter_values_detected(self, extractor):
+        """filter_values() calls are detected."""
+        result = extractor.discover_template_variables({
+            "sql": "SELECT * FROM table WHERE column IN {{ filter_values('country') }}"
+        })
+        assert any(v["variable_name"] == "country" for v in result)
+        assert any(v["variable_kind"] == "native_filter" for v in result)
+
+    def test_url_param_detected(self, extractor):
+        """url_param() calls are detected."""
+        result = extractor.discover_template_variables({
+            "sql": "SELECT * FROM table WHERE col = '{{ url_param('param_name', 'default') }}'"
+        })
+        assert any(v["variable_name"] == "param_name" for v in result)
+
+    def test_url_param_required(self, extractor):
+        """url_param without default is required."""
+        result = extractor.discover_template_variables({
+            "sql": "SELECT * FROM table WHERE col = '{{ url_param('required_param') }}'"
+        })
+        param = next(v for v in result if v["variable_name"] == "required_param")
+        assert param["is_required"] is True
+
+    def test_jinja_expression_detected(self, extractor):
+        """Raw Jinja expressions are detected."""
+        result = extractor.discover_template_variables({
+            "sql": "SELECT * FROM table WHERE col = '{{ table_name }}'"
+        })
+        assert any(v["variable_name"] == "table_name" for v in result)
+
+    def test_jinja_derived_kind(self, extractor):
+        """Jinja with dot or pipe is 'derived' kind."""
+        result = extractor.discover_template_variables({
+            "sql": "SELECT * FROM table WHERE col = '{{ value|default(0) }}'"
+        })
+        derived = next(v for v in result if v["variable_name"] == "value")
+        assert derived["variable_kind"] == "derived"
+
+    def test_empty_payload(self, extractor):
+        """Empty payload returns empty list."""
+        result = extractor.discover_template_variables({})
+        assert result == []
+
+    def test_multiple_sources(self, extractor):
+        """Multiple query-bearing fields are scanned."""
+        result = extractor.discover_template_variables({
+            "sql": "{{ filter_values('country') }}",
+            "template_sql": "{{ url_param('limit') }}",
+        })
+        names = {v["variable_name"] for v in result}
+        assert "country" in names
+        assert "limit" in names
+
+    def test_deduplicated_across_sources(self, extractor):
+        """Same variable in multiple sources is deduplicated."""
+        result = extractor.discover_template_variables({
+            "sql": "{{ filter_values('country') }}",
+            "template_sql": "{{ filter_values('country') }}",
+        })
+        country_vars = [v for v in result if v["variable_name"] == "country"]
+        assert len(country_vars) == 1