diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9e75e40a..91643664 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,7 +44,7 @@ jobs: run: uv run ruff format --check . --exclude docs/_extensions --exclude sidemantic-duckdb/extension-ci-tools --exclude sidemantic-duckdb/scripts --exclude sidemantic-duckdb/duckdb --exclude sidemantic/adapters/malloy_grammar --exclude sidemantic/adapters/holistics_grammar - name: Run tests - run: uv run pytest -v --cov=sidemantic --cov-report=term-missing + run: uv run pytest -v update-schema: name: Update JSON Schema diff --git a/README.md b/README.md index 514c5bf5..b6484e30 100644 --- a/README.md +++ b/README.md @@ -353,3 +353,5 @@ Sidemantic is an ambitious but young semantic layer project. You could encounter ```bash uv run pytest -v ``` + +This prints line coverage for `sidemantic` with missing lines in the terminal. diff --git a/pyproject.toml b/pyproject.toml index c471ebd1..57174d48 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -170,7 +170,10 @@ pythonpath = ["."] markers = [ "integration: marks tests as integration tests requiring external services (deselect with '-m \"not integration\"')", ] -addopts = "-m 'not integration'" # Skip integration tests by default +addopts = "-m 'not integration' --cov=sidemantic --cov-report=term-missing" # Skip integration tests by default and show coverage + +[tool.coverage.run] +source = ["sidemantic"] [tool.uv] prerelease = "if-necessary" @@ -191,6 +194,7 @@ dev = [ "pyarrow>=14.0.0", "pygls>=2.0.0", "pytest>=8.4.2", + "pytest-cov>=5.0.0", "ruff>=0.14.0", "uvicorn>=0.34.0", ] diff --git a/sidemantic/core/semantic_layer.py b/sidemantic/core/semantic_layer.py index e27a69f2..c41d51b3 100644 --- a/sidemantic/core/semantic_layer.py +++ b/sidemantic/core/semantic_layer.py @@ -142,6 +142,15 @@ def __exit__(self, exc_type, exc_val, exc_tb): if hasattr(self.adapter, "close"): self.adapter.close() + @property + def adapter(self): + """Database adapter accessor with legacy _adapter compatibility.""" + return self._adapter + + @adapter.setter + def adapter(self, value): + self._adapter = value + @property def conn(self): """Get raw database connection for backward compatibility.""" diff --git a/sidemantic/server/server.py b/sidemantic/server/server.py index 7b227b2a..8e0e13ba 100644 --- a/sidemantic/server/server.py +++ b/sidemantic/server/server.py @@ -7,6 +7,10 @@ from sidemantic.server.connection import SemanticLayerConnection +def _sql_string_literal(value: str) -> str: + return "'" + value.replace("'", "''") + "'" + + def map_type(duckdb_type: str) -> str: """Map DuckDB types to PostgreSQL types.""" type_lower = duckdb_type.lower() @@ -72,11 +76,10 @@ def __init__(self, connection_id, executor): for schema_name, table_name in tbls: server._server.register_schema("sidemantic", schema_name) - # Use parameterized query to handle names with special characters (e.g., quotes) - cols_info = layer.adapter.raw_connection.execute( + cols_info = layer.adapter.execute( "SELECT column_name, data_type, is_nullable FROM information_schema.columns " - "WHERE table_schema = ? AND table_name = ?", - [schema_name, table_name], + f"WHERE table_schema = {_sql_string_literal(schema_name)} " + f"AND table_name = {_sql_string_literal(table_name)}" ).fetchall() columns = [] for col_name, data_type, is_nullable in cols_info: @@ -115,7 +118,7 @@ def __init__(self, connection_id, executor): # Also register the magic 'metrics' table if there are graph-level metrics if layer.graph.metrics: metric_columns = [] - for metric in layer.graph.metrics: + for metric in layer.graph.metrics.values(): metric_columns.append({metric.name: {"type": "numeric", "nullable": True}}) # Add all dimension columns from all models diff --git a/tests/db/test_adbc_adapter.py b/tests/db/test_adbc_adapter.py index 75035e32..32b3e93b 100644 --- a/tests/db/test_adbc_adapter.py +++ b/tests/db/test_adbc_adapter.py @@ -7,6 +7,8 @@ The tests will use whichever method has SQLite available. """ +from types import SimpleNamespace + import pytest # Check if adbc_driver_manager is available @@ -417,3 +419,259 @@ def test_adbc_url_path_based_uri(): adapter2 = ADBCAdapter.from_url(f"adbc://{driver_for_url}/:memory:") assert adapter2.dialect == "sqlite" adapter2.close() + + +class _FakeCursor: + def __init__(self, rows=None, description=None, arrow_table=None, close_error=False): + self.rows = list(rows or []) + self.description = description or [("value",)] + self.arrow_table = arrow_table + self.close_error = close_error + self.closed = False + + def fetchone(self): + return self.rows[0] if self.rows else None + + def fetchall(self): + return list(self.rows) + + def close(self): + self.closed = True + if self.close_error: + raise RuntimeError("close failed") + + def fetch_arrow_table(self): + return self.arrow_table + + +def test_adbc_result_fetch_helpers_close_cursor(): + import pyarrow as pa + + from sidemantic.db.adbc import ADBCResult + + cursor = _FakeCursor(rows=[(1,)], description=[("x",)], arrow_table=pa.table({"x": [1]})) + result = ADBCResult(cursor) + assert result.description == [("x",)] + assert result.fetchone() == (1,) + assert cursor.closed is True + + cursor2 = _FakeCursor(rows=[(1,), (2,)]) + result2 = ADBCResult(cursor2) + assert result2.fetchall() == [(1,), (2,)] + assert cursor2.closed is True + + cursor3 = _FakeCursor(arrow_table=pa.table({"x": [1, 2]}), close_error=True) + result3 = ADBCResult(cursor3) + batch_reader = result3.fetch_record_batch() + assert batch_reader.read_all().to_pylist() == [{"x": 1}, {"x": 2}] + assert cursor3.closed is True + + +def test_adbc_adapter_get_tables_uses_native_metadata(): + from sidemantic.db.adbc import ADBCAdapter + + adapter = ADBCAdapter.__new__(ADBCAdapter) + adapter.conn = SimpleNamespace( + adbc_get_objects=lambda: SimpleNamespace( + read_all=lambda: SimpleNamespace( + to_pydict=lambda: { + "catalog_db_schemas": [ + [ + { + "db_schema_name": "analytics", + "db_schema_tables": [{"table_name": "orders"}, {"table_name": "customers"}], + } + ] + ] + } + ) + ) + ) + + tables = adapter.get_tables() + + assert tables == [ + {"table_name": "orders", "schema": "analytics"}, + {"table_name": "customers", "schema": "analytics"}, + ] + + +def test_adbc_adapter_get_tables_falls_back_to_information_schema(monkeypatch): + from sidemantic.db.adbc import ADBCAdapter + + adapter = ADBCAdapter.__new__(ADBCAdapter) + adapter.conn = SimpleNamespace(adbc_get_objects=lambda: (_ for _ in ()).throw(RuntimeError("no metadata"))) + captured = {} + + class FakeResult: + def fetchall(self): + return [("orders", "analytics"), ("customers", "public")] + + def fake_execute(sql): + captured["sql"] = sql + return FakeResult() + + adapter.execute = fake_execute + + tables = adapter.get_tables() + + assert "information_schema.tables" in captured["sql"] + assert tables == [ + {"table_name": "orders", "schema": "analytics"}, + {"table_name": "customers", "schema": "public"}, + ] + + +def test_adbc_adapter_get_columns_uses_table_schema(): + import pyarrow as pa + + from sidemantic.db.adbc import ADBCAdapter + + adapter = ADBCAdapter.__new__(ADBCAdapter) + adapter._driver_name = "sqlite" + adapter.conn = SimpleNamespace( + adbc_get_table_schema=lambda **kwargs: pa.schema([("id", pa.int64()), ("name", pa.string())]) + ) + + columns = adapter.get_columns("orders") + + assert columns == [ + {"column_name": "id", "data_type": "int64"}, + {"column_name": "name", "data_type": "string"}, + ] + + +def test_adbc_adapter_get_columns_uses_objects_metadata_fallback(): + from sidemantic.db.adbc import ADBCAdapter + + adapter = ADBCAdapter.__new__(ADBCAdapter) + adapter._driver_name = "sqlite" + adapter.conn = SimpleNamespace( + adbc_get_table_schema=lambda **kwargs: (_ for _ in ()).throw(RuntimeError("no schema")), + adbc_get_objects=lambda **kwargs: SimpleNamespace( + read_all=lambda: SimpleNamespace( + to_pydict=lambda: { + "catalog_db_schemas": [ + [ + { + "db_schema_name": "main", + "db_schema_tables": [ + { + "table_name": "orders", + "table_columns": [ + {"column_name": "id", "xdbc_type_name": "INTEGER"}, + {"column_name": "name", "xdbc_type_name": "TEXT"}, + ], + } + ], + } + ] + ] + } + ) + ), + ) + + columns = adapter.get_columns("orders", schema="main") + + assert columns == [ + {"column_name": "id", "data_type": "INTEGER"}, + {"column_name": "name", "data_type": "TEXT"}, + ] + + +def test_adbc_adapter_get_columns_falls_back_to_sql_for_snowflake(monkeypatch): + from sidemantic.db.adbc import ADBCAdapter + + adapter = ADBCAdapter.__new__(ADBCAdapter) + adapter._driver_name = "snowflake" + adapter.conn = SimpleNamespace( + adbc_get_table_schema=lambda **kwargs: (_ for _ in ()).throw(RuntimeError("no schema")), + adbc_get_objects=lambda **kwargs: (_ for _ in ()).throw(RuntimeError("no objects")), + ) + captured = {} + + class FakeResult: + def fetchall(self): + return [("ID", "NUMBER"), ("STATUS", "VARCHAR")] + + def fake_execute(sql): + captured["sql"] = sql + return FakeResult() + + adapter.execute = fake_execute + + columns = adapter.get_columns("orders", schema="analytics") + + assert "table_name IN ('ORDERS', 'orders')" in captured["sql"] + assert "table_schema IN ('ANALYTICS', 'analytics')" in captured["sql"] + assert columns == [ + {"column_name": "ID", "data_type": "NUMBER"}, + {"column_name": "STATUS", "data_type": "VARCHAR"}, + ] + + +def test_adbc_adapter_dialect_strips_package_prefix(): + from sidemantic.db.adbc import ADBCAdapter + + adapter = ADBCAdapter.__new__(ADBCAdapter) + adapter._driver_name = "adbc_driver_postgresql" + + assert adapter.dialect == "postgres" + + +def test_adbc_adapter_close_calls_connection(): + from sidemantic.db.adbc import ADBCAdapter + + closed = {"value": False} + adapter = ADBCAdapter.__new__(ADBCAdapter) + adapter.conn = SimpleNamespace(close=lambda: closed.__setitem__("value", True)) + + adapter.close() + + assert closed["value"] is True + + +def test_adbc_adapter_from_url_sqlite_defaults_to_memory(monkeypatch): + from sidemantic.db.adbc import ADBCAdapter + + captured = {} + original_init = ADBCAdapter.__init__ + + def fake_init(self, driver, uri=None, **kwargs): + captured["driver"] = driver + captured["uri"] = uri + captured["kwargs"] = kwargs + + monkeypatch.setattr(ADBCAdapter, "__init__", fake_init) + try: + adapter = ADBCAdapter.from_url("adbc://sqlite") + finally: + monkeypatch.setattr(ADBCAdapter, "__init__", original_init) + + assert isinstance(adapter, ADBCAdapter) + assert captured["driver"] == "sqlite" + assert captured["uri"] == ":memory:" + + +def test_adbc_adapter_from_url_adbc_query_params_become_db_kwargs(monkeypatch): + from sidemantic.db.adbc import ADBCAdapter + + captured = {} + original_init = ADBCAdapter.__init__ + + def fake_init(self, driver, uri=None, **kwargs): + captured["driver"] = driver + captured["uri"] = uri + captured["kwargs"] = kwargs + + monkeypatch.setattr(ADBCAdapter, "__init__", fake_init) + try: + adapter = ADBCAdapter.from_url("adbc://snowflake?account=myacct&warehouse=wh") + finally: + monkeypatch.setattr(ADBCAdapter, "__init__", original_init) + + assert isinstance(adapter, ADBCAdapter) + assert captured["driver"] == "snowflake" + assert captured["uri"] is None + assert captured["kwargs"]["db_kwargs"] == {"account": "myacct", "warehouse": "wh"} diff --git a/tests/optional_dep_stubs.py b/tests/optional_dep_stubs.py new file mode 100644 index 00000000..a3906810 --- /dev/null +++ b/tests/optional_dep_stubs.py @@ -0,0 +1,89 @@ +"""Helpers to stub optional dependencies in unit tests.""" + +from __future__ import annotations + +import sys +import types + + +def ensure_fake_mcp() -> None: + """Install a tiny MCP stub when the optional dependency is unavailable.""" + try: + import mcp.server.fastmcp # noqa: F401 + + return + except Exception: + pass + + mcp_module = types.ModuleType("mcp") + server_module = types.ModuleType("mcp.server") + fastmcp_module = types.ModuleType("mcp.server.fastmcp") + + class FastMCP: + def __init__(self, *_args, **_kwargs): + self.settings = types.SimpleNamespace(port=None) + + def tool(self, *args, **kwargs): + def decorator(fn): + return fn + + return decorator + + def resource(self, *args, **kwargs): + def decorator(fn): + return fn + + return decorator + + def run(self, *args, **kwargs): + return None + + fastmcp_module.FastMCP = FastMCP + server_module.fastmcp = fastmcp_module + mcp_module.server = server_module + + sys.modules.setdefault("mcp", mcp_module) + sys.modules.setdefault("mcp.server", server_module) + sys.modules.setdefault("mcp.server.fastmcp", fastmcp_module) + + +def ensure_fake_riffq() -> None: + """Install a tiny riffq stub when the optional dependency is unavailable.""" + try: + import riffq # noqa: F401 + + return + except Exception: + pass + + riffq_module = types.ModuleType("riffq") + + class BaseConnection: + def __init__(self, connection_id, executor): + self.connection_id = connection_id + self.executor = executor + + def send_reader(self, reader, callback): + callback(True) + + class _InnerServer: + def register_database(self, *args, **kwargs): + return None + + def register_schema(self, *args, **kwargs): + return None + + def register_table(self, *args, **kwargs): + return None + + class RiffqServer: + def __init__(self, *args, **kwargs): + self._server = _InnerServer() + + def start(self, **kwargs): + return None + + riffq_module.BaseConnection = BaseConnection + riffq_module.RiffqServer = RiffqServer + + sys.modules.setdefault("riffq", riffq_module) diff --git a/tests/server/test_server_catalog.py b/tests/server/test_server_catalog.py index e23503e0..4140e303 100644 --- a/tests/server/test_server_catalog.py +++ b/tests/server/test_server_catalog.py @@ -1,12 +1,12 @@ """Tests for server catalog registration.""" -import pytest - from sidemantic import Dimension, Metric, Model, SemanticLayer +from tests.optional_dep_stubs import ensure_fake_riffq + +ensure_fake_riffq() def test_map_type(): - pytest.importorskip("riffq") from sidemantic.server.server import map_type assert map_type("INTEGER") == "integer" @@ -19,11 +19,10 @@ def test_map_type(): assert map_type("DATE") == "date" assert map_type("TIMESTAMP") == "timestamp" assert map_type("BOOLEAN") == "boolean" + assert map_type("JSON") == "text" def test_start_server_registers_tables(monkeypatch): - pytest.importorskip("riffq") - from sidemantic.server.server import start_server calls = {"schemas": set(), "tables": []} @@ -64,3 +63,69 @@ def start(self, *args, **kwargs): assert ("sidemantic", "semantic_layer") in calls["schemas"] assert any(t[2] == "source_table" for t in calls["tables"]) assert any(t[2] == "orders" and t[1] == "semantic_layer" for t in calls["tables"]) + + +def test_start_server_registers_metrics_table_and_binds_connection(monkeypatch): + from sidemantic.server.server import start_server + + calls = {"tables": [], "start": None, "bound": None} + + class FakeSemanticLayerConnection: + def __init__(self, connection_id, executor, layer, username, password): + calls["bound"] = { + "connection_id": connection_id, + "executor": executor, + "layer": layer, + "username": username, + "password": password, + } + + class FakeInnerServer: + def register_database(self, name): + self.database = name + + def register_schema(self, db, schema): + pass + + def register_table(self, db, schema, table, columns): + calls["tables"].append((db, schema, table, columns)) + + class FakeServer: + def __init__(self, address, connection_cls): + self.address = address + self._server = FakeInnerServer() + connection_cls(9, "executor") + + def start(self, **kwargs): + calls["start"] = kwargs + + monkeypatch.setattr("sidemantic.server.server.SemanticLayerConnection", FakeSemanticLayerConnection) + monkeypatch.setattr("riffq.RiffqServer", FakeServer) + + layer = SemanticLayer(connection="duckdb:///:memory:") + layer.adapter.execute("CREATE TABLE source_table (id INTEGER, created_at TIMESTAMP, is_active BOOLEAN)") + layer.add_model( + Model( + name="orders", + table="source_table", + primary_key="id", + dimensions=[ + Dimension(name="created_at", sql="created_at", type="time", granularity="day"), + Dimension(name="is_active", sql="is_active", type="boolean"), + ], + metrics=[Metric(name="order_count", agg="count")], + ) + ) + layer.add_metric(Metric(name="global_ratio", type="derived", sql="1")) + + start_server(layer, host="0.0.0.0", port=5546, username="api-user", password="api-pass") + + metrics_table = next(t for t in calls["tables"] if t[2] == "metrics") + metric_columns = metrics_table[3] + assert any("global_ratio" in column for column in metric_columns) + assert any("created_at" in column and column["created_at"]["type"] == "timestamp" for column in metric_columns) + assert any("is_active" in column and column["is_active"]["type"] == "boolean" for column in metric_columns) + assert calls["bound"]["layer"] is layer + assert calls["bound"]["username"] == "api-user" + assert calls["bound"]["password"] == "api-pass" + assert calls["start"] == {"catalog_emulation": False} diff --git a/tests/test_cli_commands.py b/tests/test_cli_commands.py index 820416bc..4e61c209 100644 --- a/tests/test_cli_commands.py +++ b/tests/test_cli_commands.py @@ -2,10 +2,14 @@ from pathlib import Path +import duckdb import pytest from typer.testing import CliRunner +import sidemantic.cli as cli_module from sidemantic.cli import app +from sidemantic.config import SidemanticConfig +from tests.optional_dep_stubs import ensure_fake_mcp, ensure_fake_riffq runner = CliRunner() @@ -29,6 +33,72 @@ def _write_min_model(directory: Path) -> None: ) +def _write_orders_db(path: Path) -> None: + conn = duckdb.connect(str(path)) + conn.execute("CREATE TABLE orders (id INTEGER, status VARCHAR)") + conn.execute("INSERT INTO orders VALUES (1, 'completed'), (2, 'pending')") + conn.close() + + +def test_version_option_prints_version(): + result = runner.invoke(app, ["--version"]) + + assert result.exit_code == 0 + assert "sidemantic " in result.stdout + + +def test_info_prints_model_summary(tmp_path): + _write_min_model(tmp_path) + + result = runner.invoke(app, ["info", str(tmp_path)]) + + assert result.exit_code == 0 + assert "Semantic Layer:" in result.stdout + assert "orders" in result.stdout + assert "Dimensions: 1" in result.stdout + assert "Metrics: 1" in result.stdout + + +def test_query_dry_run_emits_sql(tmp_path): + _write_min_model(tmp_path) + + result = runner.invoke( + app, ["query", "SELECT order_count, status FROM orders", "--models", str(tmp_path), "--dry-run"] + ) + + assert result.exit_code == 0 + assert "select" in result.stdout.lower() + assert "count" in result.stdout.lower() + + +def test_query_writes_csv_using_autodetected_data_db(tmp_path): + models_dir = tmp_path / "models" + _write_min_model(models_dir) + data_dir = models_dir / "data" + data_dir.mkdir() + _write_orders_db(data_dir / "warehouse.db") + output_path = tmp_path / "results.csv" + + result = runner.invoke( + app, + [ + "query", + "SELECT status, order_count FROM orders ORDER BY status", + "--models", + str(models_dir), + "--output", + str(output_path), + ], + ) + + assert result.exit_code == 0 + assert output_path.exists() + output = output_path.read_text() + assert "status,order_count" in output + assert "completed,1" in output + assert "pending,1" in output + + def test_workbench_calls_runner(monkeypatch, tmp_path): pytest.importorskip("textual") called = {} @@ -102,7 +172,7 @@ def fake_main(): def test_serve_calls_start_server(monkeypatch, tmp_path): - pytest.importorskip("riffq") + ensure_fake_riffq() called = {} def fake_start_server(layer, host, port, username, password): @@ -124,7 +194,7 @@ def fake_start_server(layer, host, port, username, password): def test_serve_rejects_partial_auth(monkeypatch, tmp_path): - pytest.importorskip("riffq") + ensure_fake_riffq() def fake_start_server(*args, **kwargs): raise AssertionError("start_server should not be called with partial auth config") @@ -138,6 +208,47 @@ def fake_start_server(*args, **kwargs): assert "both --username and --password" in result.output +def test_serve_uses_loaded_config_defaults(monkeypatch, tmp_path): + ensure_fake_riffq() + called = {} + models_dir = tmp_path / "models" + _write_min_model(models_dir) + config_path = tmp_path / "sidemantic.yaml" + config_path.write_text( + f""" +models_dir: {models_dir} +connection: + type: duckdb + path: ":memory:" +pg_server: + host: 0.0.0.0 + port: 5545 + username: config-user + password: config-pass +""" + ) + + def fake_start_server(layer, host, port, username, password): + called["layer"] = layer + called["host"] = host + called["port"] = port + called["username"] = username + called["password"] = password + + monkeypatch.setattr("sidemantic.server.server.start_server", fake_start_server) + + cli_module._loaded_config = None + result = runner.invoke(app, ["--config", str(config_path), "serve"]) + + assert result.exit_code == 0 + assert called["host"] == "0.0.0.0" + assert called["port"] == 5545 + assert called["username"] == "config-user" + assert called["password"] == "config-pass" + assert called["layer"].connection_string == "duckdb:///:memory:" + assert "Loaded config from:" in result.stderr + + def test_api_serve_calls_start_server(monkeypatch, tmp_path): pytest.importorskip("fastapi") called = {} @@ -177,6 +288,22 @@ def fake_start_api_server(layer, host, port, auth_token, cors_origins, max_reque assert called["max_request_body_bytes"] == 2048 +def test_tree_alias_calls_workbench(monkeypatch, tmp_path): + pytest.importorskip("textual") + called = {} + + def fake_run_workbench(directory): + called["directory"] = directory + + monkeypatch.setattr("sidemantic.workbench.run_workbench", fake_run_workbench) + + _write_min_model(tmp_path) + result = runner.invoke(app, ["tree", str(tmp_path)]) + + assert result.exit_code == 0 + assert called["directory"] == tmp_path + + def test_cli_source_uses_public_adapter(): """cli.py should not reference ._adapter anywhere.""" import os @@ -189,7 +316,7 @@ def test_cli_source_uses_public_adapter(): def test_mcp_serve_calls_initialize(monkeypatch, tmp_path): - pytest.importorskip("mcp") + ensure_fake_mcp() called = {} def fake_initialize_layer(directory, db_path=None, connection=None, init_sql=None): @@ -218,3 +345,91 @@ def test_docker_entrypoint_does_not_use_eval(): content = entrypoint_path.read_text() assert "eval " not in content + + +def test_mcp_serve_apps_implies_http_and_uses_config(monkeypatch, tmp_path): + ensure_fake_mcp() + called = {} + models_dir = tmp_path / "models" + _write_min_model(models_dir) + config_path = tmp_path / "sidemantic.yaml" + config_path.write_text( + """ +models_dir: . +connection: + type: duckdb + path: ":memory:" + init_sql: + - SELECT 42 +""" + ) + + def fake_initialize_layer(directory, db_path=None, connection=None, init_sql=None): + called["directory"] = directory + called["db_path"] = db_path + called["connection"] = connection + called["init_sql"] = init_sql + + def fake_run(*args, **kwargs): + called["transport"] = kwargs["transport"] + + monkeypatch.setattr("sidemantic.mcp_server.initialize_layer", fake_initialize_layer) + monkeypatch.setattr("sidemantic.mcp_server.mcp.run", fake_run) + + import sidemantic.mcp_server as mcp_mod + + mcp_mod._apps_enabled = False + cli_module._loaded_config = None + result = runner.invoke( + app, ["--config", str(config_path), "mcp-serve", str(models_dir), "--apps", "--port", "4201"] + ) + + assert result.exit_code == 0 + assert called["directory"] == str(models_dir) + assert called["connection"] == "duckdb:///:memory:" + assert called["init_sql"] == ["SELECT 42"] + assert called["transport"] == "streamable-http" + assert mcp_mod._apps_enabled is True + assert "Note: --apps implies HTTP transport" in result.stderr + + +def test_query_uses_loaded_config_init_sql(monkeypatch, tmp_path): + models_dir = tmp_path / "models" + _write_min_model(models_dir) + cli_module._loaded_config = SidemanticConfig.model_validate( + { + "models_dir": str(models_dir), + "connection": {"type": "duckdb", "path": ":memory:", "init_sql": ["select 7"]}, + } + ) + captured = {} + + class FakeResult: + description = [("order_count",), ("status",)] + + def fetchall(self): + return [(2, "completed")] + + class FakeLayer: + def __init__(self, **kwargs): + captured["kwargs"] = kwargs + self.graph = type("Graph", (), {"models": {"orders": object()}})() + + def sql(self, sql): + captured["sql"] = sql + return FakeResult() + + def fake_load_from_directory(layer, directory): + captured["directory"] = directory + + monkeypatch.setattr("sidemantic.cli.SemanticLayer", FakeLayer) + monkeypatch.setattr("sidemantic.cli.load_from_directory", fake_load_from_directory) + + result = runner.invoke(app, ["query", "SELECT order_count, status FROM orders", "--models", str(models_dir)]) + + assert result.exit_code == 0 + assert captured["kwargs"]["connection"] == "duckdb:///:memory:" + assert captured["kwargs"]["init_sql"] == ["select 7"] + assert captured["directory"] == str(models_dir) + assert "order_count,status" in result.stdout + assert "2,completed" in result.stdout diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 2c2944e2..308a9be0 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -1,15 +1,24 @@ """Tests for MCP server functionality.""" +# ruff: noqa: E402 + import json import tempfile +from datetime import date, datetime, time from decimal import Decimal from pathlib import Path import pytest -pytest.importorskip("mcp") # Skip if mcp extra not installed +from tests.optional_dep_stubs import ensure_fake_mcp + +ensure_fake_mcp() +from sidemantic import Metric, Model +from sidemantic.core.relationship import Relationship from sidemantic.mcp_server import ( + _convert_to_json_compatible, + _format_join_condition, catalog_resource, create_chart, get_models, @@ -21,6 +30,25 @@ ) +@pytest.fixture +def stub_chart_rendering(monkeypatch): + import sidemantic.mcp_server as mcp_mod + + def fake_make_chart(data, chart_type="auto", title=None, width=600, height=400): + return { + "mark": "bar" if chart_type == "auto" else chart_type, + "title": title, + "width": width, + "height": height, + "data": {"values": data}, + } + + monkeypatch.setattr("sidemantic.charts.create_chart", fake_make_chart) + monkeypatch.setattr("sidemantic.charts.chart_to_vega", lambda chart: chart) + monkeypatch.setattr("sidemantic.charts.chart_to_base64_png", lambda _chart: "data:image/png;base64,ZmFrZQ==") + monkeypatch.setattr(mcp_mod, "_apps_enabled", False) + + @pytest.fixture def demo_layer(): """Create a demo semantic layer for testing.""" @@ -259,7 +287,7 @@ def test_run_query_decimal_conversion(demo_layer): pytest.fail(f"Result is not JSON serializable: {e}") -def test_create_chart_basic(demo_layer): +def test_create_chart_basic(demo_layer, stub_chart_rendering): """Test creating a basic chart.""" result = create_chart( dimensions=["orders.customer_name"], @@ -289,7 +317,7 @@ def test_create_chart_basic(demo_layer): assert result["row_count"] == 2 # Alice and Bob -def test_create_chart_decimal_conversion(demo_layer): +def test_create_chart_decimal_conversion(demo_layer, stub_chart_rendering): """Test that create_chart converts Decimals to floats for JSON serialization.""" result = create_chart( dimensions=["orders.customer_name"], @@ -313,7 +341,7 @@ def test_create_chart_decimal_conversion(demo_layer): pytest.fail(f"create_chart result is not JSON serializable: {e}") -def test_create_chart_with_filter(demo_layer): +def test_create_chart_with_filter(demo_layer, stub_chart_rendering): """Test creating a chart with a filter.""" result = create_chart( dimensions=["orders.customer_name"], @@ -327,7 +355,7 @@ def test_create_chart_with_filter(demo_layer): assert "Alice" in result["sql"] -def test_create_chart_time_series(demo_layer): +def test_create_chart_time_series(demo_layer, stub_chart_rendering): """Test creating a time series chart.""" result = create_chart( dimensions=["orders.order_date"], @@ -585,3 +613,184 @@ def test_catalog_resource(demo_layer): col_names = [c["column_name"] for c in order_cols] assert "customer_name" in col_names assert "total_revenue" in col_names + + +def test_convert_to_json_compatible_handles_common_scalar_types(): + assert _convert_to_json_compatible(Decimal("12.5")) == 12.5 + assert _convert_to_json_compatible(date(2024, 1, 2)) == "2024-01-02" + assert _convert_to_json_compatible(time(12, 30, 45)) == "12:30:45" + assert _convert_to_json_compatible(datetime(2024, 1, 2, 3, 4, 5)) == "2024-01-02T03:04:05" + assert _convert_to_json_compatible("plain") == "plain" + + +def test_initialize_layer_builds_connection_from_db_path(tmp_path): + model_file = tmp_path / "orders.yml" + model_file.write_text( + """ +models: + - name: orders + table: orders_table + primary_key: id + dimensions: + - name: status + sql: status + type: categorical +""" + ) + + layer = initialize_layer(str(tmp_path), db_path=":memory:") + + assert layer.connection_string == "duckdb:///:memory:" + assert "orders" in layer.graph.models + + +def test_format_join_condition_handles_all_relationship_shapes(): + models = { + "orders": Model(name="orders", table="orders", primary_key="id"), + "customers": Model(name="customers", table="customers", primary_key="id"), + "line_items": Model(name="line_items", table="line_items", primary_key="id"), + "products": Model(name="products", table="products", primary_key="id"), + "order_products": Model(name="order_products", table="order_products", primary_key="id"), + } + + assert ( + _format_join_condition( + "orders", + Relationship(name="customers", type="many_to_one", foreign_key="customer_id"), + models, + ) + == "orders.customer_id = customers.id" + ) + assert ( + _format_join_condition( + "orders", + Relationship(name="line_items", type="one_to_many", foreign_key="order_id"), + models, + ) + == "line_items.order_id = orders.id" + ) + assert ( + _format_join_condition( + "orders", + Relationship( + name="products", + type="many_to_many", + through="order_products", + through_foreign_key="order_id", + related_foreign_key="product_id", + ), + models, + ) + == "orders.id = order_products.order_id AND order_products.product_id = products.id" + ) + assert _format_join_condition("orders", Relationship(name="missing", type="many_to_one"), models) is None + + +def test_get_models_includes_join_conditions_and_source_metadata(): + tmpdir = Path(tempfile.mkdtemp()) + try: + (tmpdir / "models.yml").write_text( + """ +models: + - name: customers + table: customers + primary_key: id + dimensions: + - name: name + sql: name + type: categorical + - name: orders + table: orders + primary_key: id + relationships: + - name: customers + type: many_to_one + foreign_key: customer_id + dimensions: + - name: customer_id + sql: customer_id + type: categorical +""" + ) + + layer = initialize_layer(str(tmpdir), db_path=":memory:") + layer.graph.models["orders"]._source_format = "yaml" + layer.graph.models["orders"]._source_file = str(tmpdir / "models.yml") + + result = get_models(["orders"]) + model = result["models"][0] + relationship = model["relationships"][0] + + assert relationship["join_condition"] == "orders.customer_id = customers.id" + assert model["source_format"] == "yaml" + assert model["source_file"].endswith("models.yml") + finally: + import shutil + + shutil.rmtree(tmpdir) + + +def test_get_semantic_graph_includes_joinable_pairs_and_graph_metrics(): + tmpdir = Path(tempfile.mkdtemp()) + try: + (tmpdir / "models.yml").write_text( + """ +models: + - name: customers + table: customers + primary_key: id + dimensions: + - name: name + sql: name + type: categorical + - name: orders + table: orders + primary_key: id + relationships: + - name: customers + type: many_to_one + foreign_key: customer_id + dimensions: + - name: customer_id + sql: customer_id + type: categorical + metrics: + - name: order_count + agg: count +""" + ) + layer = initialize_layer(str(tmpdir), db_path=":memory:") + layer.add_metric(Metric(name="global_ratio", type="derived", sql="1")) + + result = get_semantic_graph() + + assert any(pair["from"] == "customers" and pair["to"] == "orders" for pair in result["joinable_pairs"]) + assert any(metric["name"] == "global_ratio" for metric in result["graph_metrics"]) + finally: + import shutil + + shutil.rmtree(tmpdir) + + +def test_create_chart_returns_ui_resource_when_apps_enabled(monkeypatch, demo_layer, stub_chart_rendering): + import sidemantic.mcp_server as mcp_mod + + def fake_resource(spec): + return {"resource": spec["mark"]} + + monkeypatch.setattr("sidemantic.apps.create_chart_resource", fake_resource) + + old_value = mcp_mod._apps_enabled + mcp_mod._apps_enabled = True + try: + result = create_chart( + dimensions=["orders.customer_name"], + metrics=["orders.total_revenue"], + chart_type="bar", + ) + finally: + mcp_mod._apps_enabled = old_value + + assert isinstance(result, list) + assert result[0]["row_count"] == 2 + assert result[1] == {"resource": "bar"} diff --git a/tests/test_mcp_server_errors.py b/tests/test_mcp_server_errors.py index 57e2fc22..f7e6f934 100644 --- a/tests/test_mcp_server_errors.py +++ b/tests/test_mcp_server_errors.py @@ -1,8 +1,12 @@ """Tests for MCP server error paths and helpers.""" +# ruff: noqa: E402 + import pytest -pytest.importorskip("mcp") # Skip if mcp extra not installed +from tests.optional_dep_stubs import ensure_fake_mcp + +ensure_fake_mcp() from sidemantic import mcp_server diff --git a/uv.lock b/uv.lock index fe26cc40..1c214aa6 100644 --- a/uv.lock +++ b/uv.lock @@ -3403,6 +3403,7 @@ dev = [ { name = "pyarrow" }, { name = "pygls" }, { name = "pytest" }, + { name = "pytest-cov" }, { name = "ruff" }, { name = "uvicorn" }, ] @@ -3486,6 +3487,7 @@ dev = [ { name = "pyarrow", specifier = ">=14.0.0" }, { name = "pygls", specifier = ">=2.0.0" }, { name = "pytest", specifier = ">=8.4.2" }, + { name = "pytest-cov", specifier = ">=5.0.0" }, { name = "ruff", specifier = ">=0.14.0" }, { name = "uvicorn", specifier = ">=0.34.0" }, ]