From 3c04ce353ef6caf20dcb3e48d203b382eda58dfd Mon Sep 17 00:00:00 2001 From: AivanF Date: Fri, 24 Jul 2026 12:17:02 +0300 Subject: [PATCH 1/2] MCP tools fixes --- slayer/inspect/service.py | 15 +++++++++--- slayer/memories/resolver.py | 6 ++++- tests/test_entity_resolution.py | 12 +++++++++ tests/test_inspect.py | 43 ++++++++++++++++++++++++--------- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/slayer/inspect/service.py b/slayer/inspect/service.py index bdb132bb..e3e526a0 100644 --- a/slayer/inspect/service.py +++ b/slayer/inspect/service.py @@ -960,19 +960,26 @@ def _render_leaf_entry( full_text = self._truncate_description_field( text=entry.text, max_chars=descriptions_max_chars, ) + # Measures / aggregations carry no verbose sample data — their full text + # (formula / params / label / type) is essential, so include it even in + # compact mode. Columns stay gated: their text can carry sampled values + # (a DB read / verbose output). + show_full = bool(full_text) and ( + not compact or entity_type in {"measure", "aggregation"} + ) if fmt == "json": payload = { "canonical_id": canonical, "entity_type": entity_type, "description": trunc_desc, } - # ``text`` present iff non-empty (DEV-1588 follow-up): compact mode - # omits it; full mode carries the entity render. - if not compact and full_text: + # ``text`` present iff non-empty (DEV-1588 follow-up): included in + # full mode, and for measures/aggregations in compact mode too. + if show_full: payload["text"] = full_text payload["warnings"] = warnings return json.dumps(payload) - body = (trunc_desc or "") if compact else full_text + body = full_text if show_full else (trunc_desc or "") return self._markdown_with_warnings(body, warnings) @staticmethod diff --git a/slayer/memories/resolver.py b/slayer/memories/resolver.py index e7bbae9f..ab22c211 100644 --- a/slayer/memories/resolver.py +++ b/slayer/memories/resolver.py @@ -295,7 +295,11 @@ async def resolve_entity( # NOSONAR(S3776) — single linear dispatch matching ) segments = prefix.split(".") - if not all(re.match(r"^[a-zA-Z_]\w*$", s) for s in segments): + # Hyphens are permitted in a segment: a datasource name may embed a UUID + # (e.g. ``source_019f4323-cb59-77c3-...``), so the datasource segment can + # contain hyphens. Model/leaf segments that don't match a stored entity + # simply resolve to not-found below, so allowing the character is safe. + if not all(re.match(r"^[a-zA-Z_][\w-]*$", s) for s in segments): raise EntityResolutionError( f"'{raw}' contains an invalid identifier segment." ) diff --git a/tests/test_entity_resolution.py b/tests/test_entity_resolution.py index 774394db..1a4a6d60 100644 --- a/tests/test_entity_resolution.py +++ b/tests/test_entity_resolution.py @@ -317,6 +317,18 @@ async def test_one_dot_datasource_model( result = await resolve_entity("mydb.orders", storage=storage) assert result.canonical_forms == ["mydb.orders"] + async def test_hyphenated_datasource_segment_accepted( + self, storage: StorageBackend + ) -> None: + # A datasource name may embed a UUID (e.g. ``source_019f4323-cb59-...``), + # so the identifier guard must accept a hyphenated segment. Resolution + # then proceeds and fails on not-found — never on a rejected segment. + with pytest.raises(EntityResolutionError) as exc_info: + await resolve_entity( + "source_019f4323-cb59.nonexistent_model.amount", storage=storage + ) + assert "invalid identifier segment" not in str(exc_info.value) + async def test_two_dot_datasource_model_column( self, storage: StorageBackend ) -> None: diff --git a/tests/test_inspect.py b/tests/test_inspect.py index e2357849..292c9ae6 100644 --- a/tests/test_inspect.py +++ b/tests/test_inspect.py @@ -17,16 +17,18 @@ * The reference is normalized via ``resolve_entity`` (so join paths like ``orders.customers.region`` resolve to the owning model's canonical), and the normalized canonical id is echoed back (always in the JSON shape). -* ``compact=True`` (default): leaf (column/measure/aggregation), datasource, - and memory render description-only; the **model** kind renders a cheap - schema *skeleton* (column / measure / aggregation **names** + join targets, - zero DB calls). ``compact=False`` → full render; for the **datasource** - kind, ``compact=False`` renders a per-model skeleton for each visible model. - ``format`` is ``markdown`` (default) | ``json``. +* ``compact=True`` (default): a **column**, datasource, and memory render + description-only; **measure** / **aggregation** leaves also include their full + text (formula / params / type — no sample data, so cheap); the **model** kind + renders a cheap schema *skeleton* (column / measure / aggregation **names** + + join targets, zero DB calls). ``compact=False`` → full render; for the + **datasource** kind, ``compact=False`` renders a per-model skeleton for each + visible model. ``format`` is ``markdown`` (default) | ``json``. * JSON ``text`` is present **iff non-empty**: ``compact=True`` JSON omits the - ``text`` key entirely for every kind; ``compact=False`` JSON carries ``text`` - only where it holds a render (memory / leaf), ``models`` for the datasource - kind, and the full ``inspect_model`` payload for the model kind. + ``text`` key for columns / datasource / memory but keeps it for measures / + aggregations; ``compact=False`` JSON carries ``text`` wherever it holds a + render (memory / leaf), ``models`` for the datasource kind, and the full + ``inspect_model`` payload for the model kind. * ``inspect`` RENDERS hidden entities (deliberate escape-hatch lookup). * Model-only args (``num_rows``/``show_sql``/``sections``/ ``descriptions_max_chars``) apply where they map; otherwise ignored with a @@ -1284,19 +1286,38 @@ class TestCompactTextOmittedAllKinds: "reference,entity_type", [ ("mydb.orders.amount", "column"), - ("mydb.orders.aov", "measure"), - ("mydb.orders.big", "aggregation"), ], ) async def test_leaf_compact_json_omits_text( self, svc: InspectService, reference: str, entity_type: str ) -> None: + # Columns stay description-only in compact (their text can carry + # sampled values / DB reads). out = await svc.inspect( reference=reference, entity_type=entity_type, format="json", compact=True, ) assert "text" not in json.loads(out) + @pytest.mark.parametrize( + "reference,entity_type", + [ + ("mydb.orders.aov", "measure"), + ("mydb.orders.big", "aggregation"), + ], + ) + async def test_leaf_compact_json_keeps_text_for_measure_and_aggregation( + self, svc: InspectService, reference: str, entity_type: str + ) -> None: + # Formula / params / type are essential and carry no sample data, so + # the compact leaf render includes them for measures/aggregations. + out = await svc.inspect( + reference=reference, entity_type=entity_type, + format="json", compact=True, + ) + p = json.loads(out) + assert "text" in p and p["text"] + @pytest.mark.parametrize( "reference,entity_type", [ From 62653dcd7a347d75ec7f045d977b3ca1cb02c51e Mon Sep 17 00:00:00 2001 From: AivanF Date: Fri, 24 Jul 2026 15:49:56 +0300 Subject: [PATCH 2/2] Inspect tool label fix --- slayer/inspect/model_render.py | 8 ++++---- tests/integration/test_mcp_inspect.py | 6 +++--- tests/test_inspect.py | 2 +- tests/test_mcp_server.py | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/slayer/inspect/model_render.py b/slayer/inspect/model_render.py index d9ca70d0..a973acab 100644 --- a/slayer/inspect/model_render.py +++ b/slayer/inspect/model_render.py @@ -995,10 +995,10 @@ async def _persist_sample( sample_data = {"columns": cols, "rows": data} sample_result.columns = cols sample_result.data = data - sample_section = f"## Sample Data\n\n{sample_result.to_markdown()}" + sample_section = f"## Data Profile\n\n{sample_result.to_markdown()}" if show_sql and sample_sql: sample_section = ( - f"## Sample Data SQL\n\n```sql\n{sample_sql}\n```\n\n" + f"## Data Profile SQL\n\n```sql\n{sample_sql}\n```\n\n" + sample_section ) out_sections.append(sample_section) @@ -1008,10 +1008,10 @@ async def _persist_sample( else: err = str(e) sample_error = err - sample_section = f"## Sample Data\n\n_Error fetching sample data: {err}_" + sample_section = f"## Data Profile\n\n_Error fetching data profile: {err}_" if show_sql and sample_sql: sample_section = ( - f"## Sample Data SQL\n\n```sql\n{sample_sql}\n```\n\n" + f"## Data Profile SQL\n\n```sql\n{sample_sql}\n```\n\n" + sample_section ) out_sections.append(sample_section) diff --git a/tests/integration/test_mcp_inspect.py b/tests/integration/test_mcp_inspect.py index 9257f1ac..c69301bd 100644 --- a/tests/integration/test_mcp_inspect.py +++ b/tests/integration/test_mcp_inspect.py @@ -307,8 +307,8 @@ async def test_full_response(self, env) -> None: # Sample data table: count + amount_avg + quantity_avg + 2 dim columns # SLayer names the *:count output '_count' when grouped by dimensions. - assert "## Sample Data" in result - sample_section = result.split("## Sample Data", 1)[1] + assert "## Data Profile" in result + sample_section = result.split("## Data Profile", 1)[1] assert "_count" in sample_section assert "amount_avg" in sample_section assert "quantity_avg" in sample_section @@ -353,7 +353,7 @@ async def test_columns_only_short_circuits_samples(self, env) -> None: assert "completed" in col_section # No sample data section; the reachable-via-joins heading was removed # entirely in DEV-1560 and must never appear. - assert "## Sample Data" not in result + assert "## Data Profile" not in result assert "## Reachable" not in result # Empty list-only headings for the rest are OK (model has no measures / # aggregations / joins, so they render nothing); footer should still diff --git a/tests/test_inspect.py b/tests/test_inspect.py index 292c9ae6..f119ed8e 100644 --- a/tests/test_inspect.py +++ b/tests/test_inspect.py @@ -262,7 +262,7 @@ async def test_model_compact_is_skeleton( assert "# Model: `orders`" not in out # full-render heading assert "## Columns (" not in out # full-render columns table assert "- **data_source:**" not in out # metadata bullets - assert "## Sample Data" not in out + assert "## Data Profile" not in out # --------------------------------------------------------------------------- diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 4dd148bd..3541ba16 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -724,7 +724,7 @@ async def test_columns_only_collapses_others(self, mcp_server, storage: YAMLStor assert "`customers`, `products`" in result # Reachable fields and samples fully omitted assert "## Reachable" not in result - assert "## Sample Data" not in result + assert "## Data Profile" not in result # Footer present assert "> Sections shown: columns." in result assert "> Names-only: measures, aggregations, joins." in result