From 4deefa8b49a909da9e3c9fc6cf473ae4bdc9a89f Mon Sep 17 00:00:00 2001 From: Rohan Dsouza Date: Wed, 26 Aug 2026 11:52:13 +0530 Subject: [PATCH 1/3] feat: report which columns are searchable, and hide the vectors an index generated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part 1 of #40. Indexes are invisible to SQL — there is no `pg_indexes` and no `information_schema.indexes` — so which column is searchable was the one fact about a table an agent could not look up and had to be told when the tools were built. `hotdata_describe_tables` now annotates each column of a described table with `searchable_by`: `text relevance` where it carries a keyword index, `meaning` where it carries a vector one. The capability is named, never the mechanism — a model can act on "matches the words a value uses" and cannot act on "bm25". Only indexes the engine reports as ready are named, because a search against one still building fails after the model has committed to that route. The cost is one control-plane call per described table, and only on the per-table call. The no-argument listing stays index-free so a wide database does not become N calls. Generated vector columns are left out. A provider-backed index over `content` materialises `content_embedding` beside it, `information_schema` reports it as an ordinary column, and nothing there marks it as generated — so describing it invites queries against a float list. The text column carries the `meaning` capability instead. `SEARCH_NOUNS` and `search_nouns_by_column` name a capability on its own, where `CAPABILITY_PHRASES` and `capabilities_by_column` give it as a sentence fragment; the phrases are now derived from the nouns so the two cannot drift. Those two grouping functions were exported by the #39 work with no consumer and no test, which is what this wires up — `tests/test_indexes.py` covers them now. Verified against api.hotdata.dev, not only against mocks: - `langchain_bm25_demo.public.listings` — 85 columns, exactly one annotated - `langchain_bm25_demo.public.listing_corpus` — `content` by text relevance and `embedding` by meaning, the plain-vector-plus-BM25 shape fusion needs - `vector_spike.public.listings_semantic` — `information_schema` reports 7 columns including `content_embedding`; describe reports 6 and annotates `content`. With `search_capabilities=False` the generated column reappears. Docs corrected where this change made them false: four places asserted that nothing lets an agent discover which columns are indexed. The search tool still pins its corpus, so they now say discovery exists and the search tool has not been changed to use it, rather than claiming the un-pinning that has not happened. --- CHANGELOG.md | 27 +++++ README.md | 47 +++++++-- docs/ai-native-layer-roadmap.md | 14 ++- docs/engine-contract.md | 14 ++- hotdata_langchain/__init__.py | 4 + hotdata_langchain/indexes.py | 46 ++++++-- hotdata_langchain/schema.py | 87 ++++++++++++++- hotdata_langchain/search.py | 9 +- hotdata_langchain/tools.py | 5 + tests/test_indexes.py | 68 ++++++++++++ tests/test_schema.py | 181 ++++++++++++++++++++++++++++++++ 11 files changed, 474 insertions(+), 28 deletions(-) create mode 100644 tests/test_indexes.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ba0889..cf9315a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **`hotdata_describe_tables` reports which columns can be searched, and by what.** Each column + of a described table carries `searchable_by` — `text relevance` where it has a keyword index, + `meaning` where it has a vector one. Indexes are invisible to SQL (there is no `pg_indexes` + and no `information_schema.indexes`), so this was the one fact about a table an agent could + not look up and had to be told at construction. The capability is named rather than the + mechanism: a model can act on "matches the words a value uses", not on "bm25". + + Only indexes the engine reports as ready are named, because a search against one still + building fails after the model has committed to that route. The cost is one control-plane + call per described table, and only on the per-table call — the no-argument listing stays + index-free, so a wide database does not become N calls. `describe_search_capabilities=False` + on `make_hotdata_tools`, or `search_capabilities=False` on the lower-level helpers, turns it + off. +- A table's generated vector columns are left out of its description. Where a vector index was + built by an embedding provider over a text column, the engine materialises a vector column + beside it — `content` produces `content_embedding` — and `information_schema` reports it as an + ordinary column with nothing marking it as generated. Describing a 1536-wide float list as + ordinary data invites queries against it, so the text column carries the `meaning` capability + and the generated column is not listed. Turning `search_capabilities` off also stops the + filtering, since nothing then knows the column was generated. +- `SEARCH_NOUNS` and `search_nouns_by_column` name a search capability on its own, where + `CAPABILITY_PHRASES` and `capabilities_by_column` give it as a sentence fragment. The phrases + are now derived from the nouns, so a payload field and a description sentence cannot drift + into naming the same capability two ways. + ### Fixed - `pip install hotdata-langchain` now has a form that provides everything the documented diff --git a/README.md b/README.md index 04d3af5..2696d88 100644 --- a/README.md +++ b/README.md @@ -132,9 +132,8 @@ tools = hl.make_hotdata_tools(client, database_id="dbid...") # includ tools = hl.make_hotdata_tools(client, database_id="dbid...", describe_tables=False) # omitted ``` -It reads `information_schema` in whichever database the tools are scoped to, so it needs no -extra permissions. With it turned off, the SQL tool's description tells the agent to query -`information_schema` directly instead. +It reads `information_schema` in whichever database the tools are scoped to. With it turned +off, the SQL tool's description tells the agent to query `information_schema` directly instead. Each column also reports `non_null`, how many of the table's rows hold a value in it, alongside the table's `row_count`: @@ -159,6 +158,39 @@ A table that is declared on the database but has never been loaded has no column `information_schema`, which reads as a missing table. It is reported as declared and empty instead, which is the state a load fixes. +### Which columns can be searched + +Indexes are invisible to SQL — there is no `pg_indexes` and no `information_schema.indexes` — so +without asking the control plane, an agent cannot tell a searchable column from any other text +column. Describing one table reports what each column can be searched by: + +```json +{ + "table": "public.listing_corpus", + "columns": [ + {"name": "content", "type": "Utf8View", "searchable_by": ["text relevance"]}, + {"name": "embedding", "type": "List(Float32)", "searchable_by": ["meaning"]}, + {"name": "rating", "type": "Float64"} + ] +} +``` + +The capability is named, not the index behind it: `text relevance` matches the words a value +uses, `meaning` matches what it is about. Only indexes the engine reports as ready are named — +a search against one still building fails, and it fails after the model has committed to that +route. + +Where a vector index was built by an embedding provider over a text column, the engine +materialises a vector column beside it — building one over `content` produces +`content_embedding`. That column is real in `information_schema` and is not the agent's to +query, so it is left out of the description and the text column carries the `meaning` +capability instead. + +This costs one control-plane call per table described, and only on the per-table call: the +no-argument listing stays index-free, so a wide database is not N calls. Pass +`describe_search_capabilities=False` to turn it off, which also stops the generated vector +column being filtered, since nothing then knows it was generated. + ## Calling tools directly You can also invoke tools outside of an agent loop: @@ -229,9 +261,12 @@ hits = {t.name: t for t in tools}["hotdata_search_text"].invoke( ``` Rows come back ranked, each with a `score`. The agent supplies only `query` and an optional -`k`; the table and column are fixed when you build the tool. That is deliberate — nothing in -the tool surface lets an agent discover which columns are indexed, and the engine errors -outright rather than falling back to a scan when a column has no BM25 index. +`k`; the table and column are fixed when you build the tool. That is deliberate: the engine +errors outright rather than falling back to a scan when a column has no BM25 index, so a +model choosing its own corpus can pick one that cannot answer. `hotdata_describe_tables` now +reports which columns are searchable, which is what a model would need to choose from +something it read rather than guessed — the search tool has not been changed to accept that +choice yet. ### Text or meaning, decided by the index diff --git a/docs/ai-native-layer-roadmap.md b/docs/ai-native-layer-roadmap.md index 9283ebc..c08b604 100644 --- a/docs/ai-native-layer-roadmap.md +++ b/docs/ai-native-layer-roadmap.md @@ -99,8 +99,10 @@ their verification in [engine-contract.md](./engine-contract.md). ### Tier 1 — buildable now, zero blockers (this repo + `sdk-python-framework`) - [x] **BM25 tool.** Shipped as `hotdata_langchain/search.py` — `hotdata_search_text`, with the - corpus pinned at construction (nothing lets an agent discover which columns are indexed, - and the engine errors outright when one is missing). + corpus pinned at construction (the engine errors outright when the index is missing, so a + model choosing its own corpus can pick one that cannot answer). `hotdata_describe_tables` + now reports what is searchable; the search tool does not yet take that choice from the + agent. - [x] **Tool descriptions that carry the engine's contract.** The highest-leverage item found so far, and not originally on this list. A 12-token SQL description produced a failed run; the contract version produces the correct search-then-SQL path with no system @@ -197,9 +199,11 @@ Grouped by code surface: release rather than shipping two breaking versions. - **Retrieval surface** ([#39](https://github.com/hotdata-dev/hotdata-langchain/issues/39)) — **shipped**, both halves: the semantic search tool, and hybrid rank fusion over it and BM25 as a single SQL query rather than the client-side fan-out originally planned. -- **Discovery surface** ([#40](https://github.com/hotdata-dev/hotdata-langchain/issues/40)) — report which columns are searchable in `hotdata_describe_tables`. - Newly unblocked: indexes are invisible to SQL but `IndexesApi.list_indexes` returns them, so - this needs no engine change. It is what would let the search corpus stop being pinned. +- **Discovery surface** ([#40](https://github.com/hotdata-dev/hotdata-langchain/issues/40)) — part 1 **shipped**: `hotdata_describe_tables` reports each + column's `searchable_by`, read from `IndexesApi.list_indexes`, and hides the vector columns a + provider-backed index generated. No engine change was needed. Still open in that issue: + making multi-database tool sets distinguishable, and the un-pinning of the search corpus that + this now makes safe. - ~~**Tool-layer robustness**~~ ([#41](https://github.com/hotdata-dev/hotdata-langchain/issues/41)) — **done.** `with_error_feedback` and `engine_error_message` are package API, `make_hotdata_tools(handle_errors=True)` turns the wrapping on, and both the sync and async callables are wrapped — the async one is what LangChain actually calls in a diff --git a/docs/engine-contract.md b/docs/engine-contract.md index 3c67972..eef9279 100644 --- a/docs/engine-contract.md +++ b/docs/engine-contract.md @@ -355,12 +355,20 @@ hybrid/RRF/fusion across the engine finds nothing. Working in SQL: `information_schema.tables`, `information_schema.columns` (with `table_catalog`, `table_schema`, `table_name`, `column_name`, `ordinal_position`, `data_type`, `is_nullable`), `SHOW TABLES`, and `DESCRIBE `. `hotdata_langchain/schema.py` builds on -`information_schema.columns`, so it needs no extra permissions. +`information_schema.columns` for the schema itself, which needs no permission beyond the query +scope; describing one table additionally reads the control plane, below. **Indexes are not visible in SQL** — no `pg_indexes`, no `information_schema.indexes`. They are only reachable through the control plane, `IndexesApi.list_indexes(connection_id, schema, table)`, -which returns index name, type, columns and status. This is why an agent cannot currently -discover which columns are searchable, and why the search tool pins its corpus. +which returns index name, type, columns and status. `hotdata_describe_tables` makes that call per +described table, so an agent can now read which columns are searchable; the search tool still +pins its corpus at construction. + +Observed live (2026-08-26, `api.hotdata.dev`) on a provider-backed index over +`vector_spike.public.listings_semantic`: `information_schema` reports the generated +`content_embedding` as an ordinary column alongside the `content` it was derived from, and the +index reports `source_column = content`. The link between the two exists only in the index +record — nothing in `information_schema` marks the generated column as generated. ## Databases and workspaces diff --git a/hotdata_langchain/__init__.py b/hotdata_langchain/__init__.py index ca73df3..6d51751 100644 --- a/hotdata_langchain/__init__.py +++ b/hotdata_langchain/__init__.py @@ -26,6 +26,7 @@ ) from hotdata_langchain.indexes import ( CAPABILITY_PHRASES, + SEARCH_NOUNS, SEMANTIC, TEXT, SearchIndex, @@ -34,6 +35,7 @@ generated_vector_columns, indexes_for_column, list_search_indexes, + search_nouns_by_column, ) from hotdata_langchain.results import ( CLIENT_WARNING_KEY, @@ -93,6 +95,7 @@ "DISTANCE_FUNCTIONS", "RRF_K", "SCORE_COLUMN", + "SEARCH_NOUNS", "SEMANTIC", "TEXT", "DistanceMetric", @@ -133,6 +136,7 @@ "result_json", "result_payload", "result_rows_for_llm", + "search_nouns_by_column", "semantic_search_json", "vector_distance_sql", "vector_search_sql", diff --git a/hotdata_langchain/indexes.py b/hotdata_langchain/indexes.py index 1717c70..9d8942f 100644 --- a/hotdata_langchain/indexes.py +++ b/hotdata_langchain/indexes.py @@ -12,7 +12,7 @@ from __future__ import annotations import logging -from collections.abc import Iterator, Sequence +from collections.abc import Callable, Iterator, Sequence from dataclasses import dataclass from typing import Any, Final, Literal @@ -27,10 +27,16 @@ TEXT: Final[SearchKind] = "text" SEMANTIC: Final[SearchKind] = "semantic" +#: What each kind is called where the capability is named on its own — a payload field, +#: a list of what a column supports. +SEARCH_NOUNS: dict[str, str] = { + TEXT: "text relevance", + SEMANTIC: "meaning", +} + #: How each kind reads in a sentence written for a model. CAPABILITY_PHRASES: dict[str, str] = { - TEXT: "searchable by text relevance", - SEMANTIC: "searchable by meaning", + kind: f"searchable by {noun}" for kind, noun in SEARCH_NOUNS.items() } _READY = "ready" @@ -71,6 +77,11 @@ def capability(self) -> str: """Return the phrase describing this index for a model-facing sentence.""" return CAPABILITY_PHRASES[self.kind] + @property + def search_noun(self) -> str: + """Return what this index makes the column searchable by, named on its own.""" + return SEARCH_NOUNS[self.kind] + def _search_index(index: Any) -> SearchIndex | None: """Return ``index`` as a :class:`SearchIndex`, or ``None`` if it serves no search. @@ -187,6 +198,18 @@ def indexes_for_column(indexes: Sequence[SearchIndex], column: str) -> list[Sear return [index for index in indexes if index.column == column] +def _by_column( + indexes: Sequence[SearchIndex], describe: Callable[[SearchIndex], str] +) -> dict[str, list[str]]: + by_column: dict[str, list[str]] = {} + for index in indexes: + entries = by_column.setdefault(index.column, []) + described = describe(index) + if described not in entries: + entries.append(described) + return by_column + + def capabilities_by_column(indexes: Sequence[SearchIndex]) -> dict[str, list[str]]: """Return each column's search capabilities, as phrases, keyed by column name. @@ -196,12 +219,17 @@ def capabilities_by_column(indexes: Sequence[SearchIndex]) -> dict[str, list[str different columns of the same table, and a provider-backed index cannot share a table with any other index at all. """ - by_column: dict[str, list[str]] = {} - for index in indexes: - phrases = by_column.setdefault(index.column, []) - if index.capability not in phrases: - phrases.append(index.capability) - return by_column + return _by_column(indexes, lambda index: index.capability) + + +def search_nouns_by_column(indexes: Sequence[SearchIndex]) -> dict[str, list[str]]: + """Return what each column is searchable by, keyed by column name. + + The same grouping as :func:`capabilities_by_column`, naming the capability on its own + rather than as a sentence fragment, for a payload field that already says the column + is searchable. + """ + return _by_column(indexes, lambda index: index.search_noun) def generated_vector_columns(indexes: Sequence[SearchIndex]) -> Iterator[str]: diff --git a/hotdata_langchain/schema.py b/hotdata_langchain/schema.py index 7660629..35d4181 100644 --- a/hotdata_langchain/schema.py +++ b/hotdata_langchain/schema.py @@ -11,6 +11,12 @@ from hotdata_langchain._sql import quote_identifier, validate_identifier from hotdata_langchain.databases import query_scope, resolve_database_by_id +from hotdata_langchain.indexes import ( + SearchIndex, + generated_vector_columns, + list_search_indexes, + search_nouns_by_column, +) logger = logging.getLogger(__name__) @@ -121,6 +127,25 @@ def _quotable(name: str) -> bool: return True +def _table_indexes( + client: HotdataClient, + *, + schema: str, + name: str, + database: ManagedDatabase | None, +) -> list[SearchIndex]: + """Return the search indexes on one table, or none when the scope cannot answer. + + Indexes belong to a connection, so an unscoped call has nothing to ask. Schema and + table come from the rows ``information_schema`` returned rather than from re-parsing + the reference, so a name this package would not have parsed still resolves. + """ + if database is None: + logger.debug("no database scope for %s.%s; not reporting searchable columns", schema, name) + return [] + return list_search_indexes(client, table=name, schema=schema, database=database) + + def _column_stats( client: HotdataClient, *, @@ -201,6 +226,7 @@ def describe_tables_json( database_id: str | ManagedDatabase | None = None, max_columns: int = DEFAULT_MAX_COLUMNS, column_stats: bool = True, + search_capabilities: bool = True, ) -> str: """Describe the scoped database's tables, or one table's columns, as JSON. @@ -216,6 +242,15 @@ def describe_tables_json( by the instrumentation that writes them presents every one as equally available. Turn it off to describe a table without scanning it. + ``search_capabilities`` annotates each column with ``searchable_by`` — what it can be + ranked by, as ``text relevance`` or ``meaning`` — at the cost of one control-plane + call per described table. Indexes are invisible to SQL, so this is the only way an + agent can find out which column to search rather than guess. It also drops the vector + columns a provider-backed index generated: those are real columns that no caller + wrote, and describing a 1536-wide float list as ordinary data invites queries against + it. Only indexes the engine reports as ready are named, because a search against one + still building fails after the model has committed to the route. + A table that is declared on the database but has never been loaded reports no columns at all, which reads as a missing table. It is reported as declared and empty instead. @@ -268,11 +303,37 @@ def describe_tables_json( truncated = len(records) > max_columns records = records[:max_columns] qualified = f"{records[0]['table_schema']}.{records[0]['table_name']}" + indexes = ( + _table_indexes( + client, + schema=str(records[0]["table_schema"]), + name=str(records[0]["table_name"]), + database=database, + ) + if search_capabilities + else [] + ) + generated = set(generated_vector_columns(indexes)) + records = [r for r in records if str(r["column_name"]) not in generated] names = [str(r["column_name"]) for r in records] columns: list[dict[str, object]] = [ {"name": name, "type": r["data_type"]} for name, r in zip(names, records, strict=True) ] + searchable = search_nouns_by_column(indexes) + annotated = False + for entry in columns: + nouns = searchable.get(str(entry["name"])) + if nouns: + entry["searchable_by"] = nouns + annotated = True payload: dict[str, object] = {"table": qualified, "columns": columns} + if annotated: + payload["search"] = ( + "searchable_by names what a column can be ranked by: 'text relevance' matches " + "the words a value uses, 'meaning' matches what it is about. A column without " + "it can still be filtered in SQL, but a substring filter is not a search — it " + "matches only the literal characters given." + ) stats = ( _column_stats(client, table=qualified, columns=names, database=database) if column_stats @@ -297,6 +358,7 @@ def describe_tables_json( def default_describe_description( *, column_stats: bool = True, + search_capabilities: bool = True, catalogs: Sequence[str] | None = None, ) -> str: """Return the agent-facing description for the schema tool. @@ -306,6 +368,11 @@ def default_describe_description( what the model plans before it reads any rows, and a column it never asked about is a column whose emptiness it never sees. + ``search_capabilities`` adds the sentence about ``searchable_by``, for the same reason + ``column_stats`` earns one: which column is searchable cannot be read in SQL, so a + model that does not know this tool reports it has no way to find out, and picks a + column to search by guessing. + ``catalogs`` names the catalogs the tools are scoped to, and the worked example uses one only when there is exactly one to use. There is no correct constant to fall back on: an instant database answers to ``default`` and an attached source answers to its @@ -322,6 +389,13 @@ def default_describe_description( if column_stats else "" ) + searchable = ( + "A column that can be searched also reports 'searchable_by': 'text relevance' " + "matches the words a value uses, 'meaning' matches what it is about. Check it " + "before searching — which column is searchable cannot be worked out from SQL.\n" + if search_capabilities + else "" + ) known = list(catalogs or ()) full = f"'{known[0]}.public.listings'" if len(known) == 1 else "the full 'catalog.schema.table'" return ( @@ -330,6 +404,7 @@ def default_describe_description( f"table name ('listings', 'public.listings' or {full}) it " "returns that table's columns and their types.\n" f"{populated}" + f"{searchable}" "Use it whenever you are unsure a table or column exists — guessing a column " "name that is not there makes the query fail." ) @@ -343,6 +418,7 @@ def make_hotdata_describe_tables_tool( description: str | None = None, max_columns: int = DEFAULT_MAX_COLUMNS, column_stats: bool = True, + search_capabilities: bool = True, catalogs: Sequence[str] | None = None, ) -> StructuredTool: """Return a LangChain tool that reports the scoped database's tables and columns. @@ -355,6 +431,10 @@ def make_hotdata_describe_tables_tool( type, at the cost of one aggregate query per described table. Turn it off where describing a table must not scan it. + ``search_capabilities`` (on by default) reports what each column can be searched by, + at the cost of one control-plane call per described table. Turn it off to describe a + table without asking what is indexed on it. + Fails fast on a non-positive ``max_columns`` rather than at first invocation. """ if max_columns < 1: @@ -375,12 +455,17 @@ def hotdata_describe_tables(table: str | None = None) -> str: database_id=database, max_columns=max_columns, column_stats=column_stats, + search_capabilities=search_capabilities, ) return StructuredTool.from_function( func=hotdata_describe_tables, name=name, description=description - or default_describe_description(column_stats=column_stats, catalogs=catalogs), + or default_describe_description( + column_stats=column_stats, + search_capabilities=search_capabilities, + catalogs=catalogs, + ), parse_docstring=True, ) diff --git a/hotdata_langchain/search.py b/hotdata_langchain/search.py index b22f6a0..579dcf7 100644 --- a/hotdata_langchain/search.py +++ b/hotdata_langchain/search.py @@ -1113,10 +1113,11 @@ def make_hotdata_search_tool( it must be the same model the column was written with. See :func:`resolve_search_route` for why ``auto`` needs no preference rule. - The corpus is pinned here rather than chosen by the model: nothing in the tool - surface lets an agent discover which columns are searchable, and the engine errors - outright when the index is missing. The agent supplies only ``query`` and an - optional ``k``. + The corpus is pinned here rather than chosen by the model: the engine errors outright + when the index is missing, so a model choosing its own corpus can pick one that cannot + answer. ``hotdata_describe_tables`` reports which columns are searchable, but this tool + does not yet take the table and column from the agent. It supplies only ``query`` and + an optional ``k``. ``database_id`` scopes the search to one instant database, by id and never by name; it is resolved once here. Pass an already-resolved ``ManagedDatabase`` to skip the diff --git a/hotdata_langchain/tools.py b/hotdata_langchain/tools.py index b33c52c..845fe51 100644 --- a/hotdata_langchain/tools.py +++ b/hotdata_langchain/tools.py @@ -304,6 +304,7 @@ def make_hotdata_tools( search_semantic_column: str | None = None, describe_tables: bool = True, describe_column_stats: bool = True, + describe_search_capabilities: bool = True, management_tools: bool = True, handle_errors: bool = False, allow_private_hosts: bool = False, @@ -324,6 +325,9 @@ def make_hotdata_tools( ``describe_column_stats`` (on by default) has that tool also report how many rows hold a value in each column, so an empty column is visible as such rather than as an ordinary typed one; it costs one aggregate query per table described. + ``describe_search_capabilities`` (on by default) has it also report what each column + can be searched by, which is the one fact about a table that SQL cannot answer; it + costs one control-plane call per table described. ``management_tools`` (on by default) adds the three tools that work on instant databases themselves — listing, creating and loading. Turn it off for an agent that @@ -556,6 +560,7 @@ def hotdata_load_managed_table( client, database_id=database, column_stats=describe_column_stats, + search_capabilities=describe_search_capabilities, catalogs=catalogs, ) ) diff --git a/tests/test_indexes.py b/tests/test_indexes.py new file mode 100644 index 0000000..52a0a29 --- /dev/null +++ b/tests/test_indexes.py @@ -0,0 +1,68 @@ +"""What a column is searchable by, grouped for the callers that report it. + +The vocabulary is the capability, never the mechanism: these assert that the words which +reach a model say "text relevance" and "meaning" rather than "bm25" and "vector". +""" + +from __future__ import annotations + +from hotdata_langchain.indexes import ( + CAPABILITY_PHRASES, + SEARCH_NOUNS, + SEMANTIC, + TEXT, + SearchIndex, + capabilities_by_column, + search_nouns_by_column, +) + + +def text_index(column: str = "description") -> SearchIndex: + return SearchIndex(column=column, kind=TEXT, index_type="bm25", ready=True) + + +def vector(column: str = "embedding") -> SearchIndex: + return SearchIndex(column=column, kind=SEMANTIC, index_type="vector", ready=True) + + +def test_the_phrases_are_built_from_the_nouns() -> None: + """One source of truth, so a payload and a sentence cannot name a capability differently.""" + assert CAPABILITY_PHRASES.keys() == SEARCH_NOUNS.keys() + for kind, noun in SEARCH_NOUNS.items(): + assert CAPABILITY_PHRASES[kind] == f"searchable by {noun}" + + +def test_no_capability_word_names_an_index_type() -> None: + words = " ".join([*SEARCH_NOUNS.values(), *CAPABILITY_PHRASES.values()]).lower() + for mechanism in ("bm25", "vector", "hnsw", "usearch", "index"): + assert mechanism not in words + + +def test_capabilities_and_nouns_group_by_column() -> None: + indexes = [text_index("description"), vector("embedding")] + assert capabilities_by_column(indexes) == { + "description": ["searchable by text relevance"], + "embedding": ["searchable by meaning"], + } + assert search_nouns_by_column(indexes) == { + "description": ["text relevance"], + "embedding": ["meaning"], + } + + +def test_two_indexes_of_one_kind_on_a_column_are_named_once() -> None: + """The engine allows more than one index to land on a column; the capability is still one.""" + assert search_nouns_by_column([text_index(), text_index()]) == { + "description": ["text relevance"] + } + + +def test_both_kinds_on_one_column_are_both_named() -> None: + assert search_nouns_by_column([text_index("body"), vector("body")]) == { + "body": ["text relevance", "meaning"] + } + + +def test_no_indexes_means_no_columns_rather_than_an_error() -> None: + assert capabilities_by_column([]) == {} + assert search_nouns_by_column([]) == {} diff --git a/tests/test_schema.py b/tests/test_schema.py index e31806c..a55a98a 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -2,6 +2,7 @@ import json from types import SimpleNamespace +from typing import Any from unittest.mock import MagicMock import pytest @@ -18,6 +19,7 @@ table_overview_sql, ) from hotdata_langchain.tools import make_hotdata_tools +from tests.conftest import bm25_index, vector_index def result(columns: list[str], rows: list[list[object]]) -> QueryResult: @@ -525,3 +527,182 @@ def test_an_uppercased_managed_catalog_still_finds_an_unloaded_table( describe_tables_json(client, table="DEFAULT.public.customer", database_id=managed_db) ) assert "no data yet" in payload["note"] + + +MANAGED_DB = ManagedDatabase( + id="dbidsf000000000000000000000001", + description="sf_airbnb", + default_connection_id="conn_sf", +) + + +# --- Searchable columns ----------------------------------------------------------- +# +# Indexes are invisible to SQL, so the control plane is the only source for this. These +# pin what reaches the model: the capability rather than the mechanism, ready indexes +# only, and no generated vector column presented as ordinary data. + + +def _described( + search_indexes: MagicMock, + indexes: list[SimpleNamespace], + columns: QueryResult = COLUMNS, + *, + search_capabilities: bool = True, +) -> tuple[dict[str, Any], MagicMock]: + client = MagicMock() + client.execute_sql.return_value = columns + search_indexes.return_value.list_indexes.return_value = SimpleNamespace(indexes=indexes) + payload = json.loads( + describe_tables_json( + client, + table="listings", + database_id=MANAGED_DB, + column_stats=False, + search_capabilities=search_capabilities, + ) + ) + return payload, client + + +def test_a_bm25_index_reports_its_column_as_searchable_by_text_relevance( + search_indexes: MagicMock, +) -> None: + payload, _ = _described(search_indexes, [bm25_index(column="description")]) + by_name = {c["name"]: c for c in payload["columns"]} + assert by_name["description"]["searchable_by"] == ["text relevance"] + assert "searchable_by" not in by_name["id"] + + +def test_the_capability_is_named_not_the_index_type(search_indexes: MagicMock) -> None: + """'bm25' is a mechanism the model cannot act on; 'text relevance' is a capability.""" + payload, _ = _described(search_indexes, [bm25_index(column="description")]) + assert "bm25" not in json.dumps(payload).lower() + + +def test_a_provider_backed_index_marks_the_text_column_and_hides_the_vector_it_generated( + search_indexes: MagicMock, +) -> None: + """The generated column is real in information_schema and is not the model's to query.""" + columns = result( + ["table_schema", "table_name", "column_name", "data_type"], + [ + ["public", "listings", "id", "Int64"], + ["public", "listings", "description", "LargeUtf8"], + ["public", "listings", "description_embedding", "List(Float32)"], + ], + ) + payload, _ = _described( + search_indexes, + [vector_index(columns=["description_embedding"], source_column="description")], + columns=columns, + ) + names = [c["name"] for c in payload["columns"]] + assert names == ["id", "description"] + by_name = {c["name"]: c for c in payload["columns"]} + assert by_name["description"]["searchable_by"] == ["meaning"] + + +def test_a_plain_vector_index_marks_the_column_it_was_built_over(search_indexes: MagicMock) -> None: + """No source column, so the vector column is the one a query names — and it stays visible.""" + columns = result( + ["table_schema", "table_name", "column_name", "data_type"], + [["public", "listings", "embedding", "List(Float32)"]], + ) + payload, _ = _described(search_indexes, [vector_index(columns=["embedding"])], columns=columns) + assert payload["columns"] == [ + {"name": "embedding", "type": "List(Float32)", "searchable_by": ["meaning"]} + ] + + +def test_an_index_still_building_is_not_reported_as_searchable(search_indexes: MagicMock) -> None: + """A search against a pending index fails after the model has committed to the route.""" + payload, _ = _described(search_indexes, [bm25_index(column="description", status="building")]) + assert all("searchable_by" not in c for c in payload["columns"]) + assert "search" not in payload + + +def test_the_note_explaining_searchable_by_appears_only_when_something_carries_it( + search_indexes: MagicMock, +) -> None: + with_index, _ = _described(search_indexes, [bm25_index(column="description")]) + without, _ = _described(search_indexes, []) + assert "text relevance" in with_index["search"] + assert "search" not in without + + +def test_search_capabilities_off_asks_the_control_plane_nothing(search_indexes: MagicMock) -> None: + payload, _ = _described( + search_indexes, [bm25_index(column="description")], search_capabilities=False + ) + search_indexes.return_value.list_indexes.assert_not_called() + assert all("searchable_by" not in c for c in payload["columns"]) + + +def test_an_unscoped_describe_reports_no_capabilities_rather_than_failing( + search_indexes: MagicMock, +) -> None: + """Indexes belong to a connection, so there is nothing to ask without a scope.""" + client = MagicMock() + client.execute_sql.return_value = COLUMNS + payload = json.loads(describe_tables_json(client, table="listings", column_stats=False)) + search_indexes.return_value.list_indexes.assert_not_called() + assert all("searchable_by" not in c for c in payload["columns"]) + + +def test_the_table_overview_makes_no_index_call(search_indexes: MagicMock) -> None: + """One call per table would be N calls for a wide database, for a listing that has no room.""" + client = MagicMock() + client.execute_sql.return_value = OVERVIEW + describe_tables_json(client, database_id=MANAGED_DB) + search_indexes.return_value.list_indexes.assert_not_called() + + +def test_a_generated_vector_column_is_not_counted_either(search_indexes: MagicMock) -> None: + """It is dropped before the stats aggregate, so nothing scans a 1536-wide float list.""" + client = MagicMock() + client.execute_sql.return_value = result( + ["table_schema", "table_name", "column_name", "data_type"], + [ + ["public", "listings", "description", "LargeUtf8"], + ["public", "listings", "description_embedding", "List(Float32)"], + ], + ) + search_indexes.return_value.list_indexes.return_value = SimpleNamespace( + indexes=[vector_index(columns=["description_embedding"], source_column="description")] + ) + describe_tables_json(client, table="listings", database_id=MANAGED_DB) + stats_sql = executed_sqls(client)[-1] + assert "description_embedding" not in stats_sql + assert "description" in stats_sql + + +def test_the_description_tells_the_model_the_tool_reports_searchability() -> None: + """A model that does not know this tool answers the question never asks it.""" + described = default_describe_description() + assert "searchable_by" in described + assert "text relevance" in described and "meaning" in described + assert "searchable_by" not in default_describe_description(search_capabilities=False) + + +def test_the_description_never_names_the_index_type() -> None: + assert "bm25" not in default_describe_description().lower() + + +def test_the_tool_set_can_turn_searchability_off(search_indexes: MagicMock) -> None: + client = MagicMock() + client.execute_sql.return_value = COLUMNS + search_indexes.return_value.list_indexes.return_value = SimpleNamespace( + indexes=[bm25_index(column="description")] + ) + tools = { + t.name: t + for t in make_hotdata_tools( + client, database_id=MANAGED_DB, describe_search_capabilities=False + ) + } + describe = tools["hotdata_describe_tables"] + assert "searchable_by" not in (describe.description or "") + payload = json.loads(describe.invoke({"table": "listings"})) + search_indexes.return_value.list_indexes.assert_not_called() + assert all("searchable_by" not in c for c in payload["columns"]) From b7e9dc726d914fd521634f2cb67c9ec5a0687a2b Mon Sep 17 00:00:00 2001 From: Rohan Dsouza Date: Wed, 26 Aug 2026 17:10:52 +0530 Subject: [PATCH 2/3] feat: name a database in its tools' descriptions, and let a tool set be suffixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part 2 of #40, closing it. One client can query many databases, so registering several tool sets in one agent is a supported shape — but two sets both registered `hotdata_execute_sql`, with byte-identical descriptions. Reproduced before fixing: two default sets give 5 distinct names for 10 tools. `make_hotdata_tools` takes `tool_name_suffix=`, appended to every name in the set. Cross-references follow it, so the SQL tool points at the schema tool of its own set rather than the other's — the load tool's description hardcoded two management tool names and would otherwise have dangled. The suffix is validated at build time (letters, digits, underscores or hyphens; result within the 64-character limit tool-calling APIs impose) rather than when a provider first rejects a call. An explicit `search_tool_name` is used exactly as given, since naming that tool is already the caller's decision. `label=` names the database at the front of the SQL, schema and search descriptions, defaulting to the database record's name. The instant-database tools deliberately do not carry it: they act on the workspace, so naming one database in them would be false. A database with no name gets no sentence rather than one naming its id — an id presented as a name invites passing it where a name is expected, which is the trap #86 removed elsewhere. `scoped_description` and `database_label` live in databases.py because search.py and schema.py both need them and both already import from there; tools.py imports from both, so the helper could not live in tools.py. The suffixed builds are added to `_tool_builds`, so all three existing description guards cover them. Both cross-reference regressions above were mutation-tested: each fails only the suffixed builds, which is what earns them a place in the matrix. Fixed a defect in the dangling-reference guard itself, found by running the real thing against production rather than mocks: its `hotdata_[a-z_]+` pattern stops at a digit, so a set suffixed `f1` had `hotdata_describe_tables_f1` truncated to `..._f` and a correct reference reported as dangling. Digits and hyphens are legal in a tool name. A `f1` build now pins it, and the old pattern fails that build. Verified against api.hotdata.dev: two sets over langchain_bm25_demo and f1_db give 8 distinct names of 8, no dangling references, each SQL tool opening with its own database's name. --- CHANGELOG.md | 15 +++++ README.md | 37 +++++++++++ docs/ai-native-layer-roadmap.md | 10 +-- hotdata_langchain/__init__.py | 2 + hotdata_langchain/databases.py | 24 ++++++++ hotdata_langchain/schema.py | 23 +++++-- hotdata_langchain/search.py | 28 ++++++--- hotdata_langchain/tools.py | 89 ++++++++++++++++++++++----- tests/test_descriptions.py | 34 ++++++++++- tests/test_tools.py | 105 ++++++++++++++++++++++++++++++++ 10 files changed, 333 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf9315a..a1e22ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ordinary data invites queries against it, so the text column carries the `meaning` capability and the generated column is not listed. Turning `search_capabilities` off also stops the filtering, since nothing then knows the column was generated. +- **Two tool sets over different databases can be told apart.** `make_hotdata_tools` takes + `tool_name_suffix=`, appended to every tool name in the set, so registering a second set no + longer puts two tools called `hotdata_execute_sql` in one prompt — a name the model can + neither address nor choose between. Cross-references between descriptions follow the suffix, + so the SQL tool points at the schema tool of its own set. The suffix is validated when the set + is built (letters, digits, underscores or hyphens; the result within the 64-character limit + tool-calling APIs impose) rather than when a provider first rejects a call. An explicit + `search_tool_name` is used exactly as given, since naming that tool is already the caller's + decision. `suffixed_tool_name` is exported for callers registering tools of their own + alongside these. +- **Descriptions name the database they work on.** The SQL, schema and search tools now open + with `Works on the 'sales' database.`, taken from the database record and overridable with + `label=`. The instant-database tools deliberately do not: they act on the workspace, so + naming one database in them would be false. A database with no name gets no such sentence + rather than one naming its id, which would present an id where a name is expected. - `SEARCH_NOUNS` and `search_nouns_by_column` name a search capability on its own, where `CAPABILITY_PHRASES` and `capabilities_by_column` give it as a sentence fragment. The phrases are now derived from the nouns, so a payload field and a description sentence cannot drift diff --git a/README.md b/README.md index 2696d88..c442a9f 100644 --- a/README.md +++ b/README.md @@ -638,6 +638,43 @@ the same id or record. Passing the record skips the lookup an id costs on each c print(hl.execute_sql_json(client, "SELECT * FROM orders LIMIT 5", database_id="dbid...")) ``` +## Giving an agent more than one database + +One client can query many databases, so registering several tool sets in one agent works. Each +set needs a name suffix, or every set registers `hotdata_execute_sql` and the model is handed +tools it cannot tell apart or address: + +```python +sales = hl.make_hotdata_tools(client, database_id="dbid...", tool_name_suffix="sales") +support = hl.make_hotdata_tools( + client, database_id="dbid...", tool_name_suffix="support", management_tools=False +) + +agent = create_agent(model=your_model, tools=[*sales, *support]) +``` + +That gives `hotdata_execute_sql_sales` and `hotdata_execute_sql_support`, and every +cross-reference between descriptions follows — the SQL tool points at the schema tool of its own +set, not the other's. A suffix must be letters, digits, underscores or hyphens, and the whole +name has to stay within 64 characters — the shortest tool-name limit among the providers this +package is used with. Both are checked when you build the set rather than when a provider +first rejects a call. + +Descriptions name their database too, so the model has something to choose on rather than two +identically-worded tools: + +> Works on the 'sales' database. Run a read-only SQL query and return the rows as JSON. … + +The name comes from the database record; pass `label=` to override it. A database with no name +gets no such sentence rather than one naming its id. Only the database-scoped tools carry it — +the instant-database tools act on the workspace, so naming one database in them would be false. + +`management_tools=False` on the extra sets is worth it for the same reason: listing, creating +and loading databases are workspace-wide, so a second copy of them is redundant surface. + +Note that a query cannot reach across databases: `SELECT ... FROM other_db.public.t` from within +one database's scope fails with `table not found`. Each set queries its own. + ## Controlling result size Limit how many rows are returned to the LLM. Useful for keeping responses within context limits (default: 100): diff --git a/docs/ai-native-layer-roadmap.md b/docs/ai-native-layer-roadmap.md index c08b604..7b6e5d2 100644 --- a/docs/ai-native-layer-roadmap.md +++ b/docs/ai-native-layer-roadmap.md @@ -199,11 +199,13 @@ Grouped by code surface: release rather than shipping two breaking versions. - **Retrieval surface** ([#39](https://github.com/hotdata-dev/hotdata-langchain/issues/39)) — **shipped**, both halves: the semantic search tool, and hybrid rank fusion over it and BM25 as a single SQL query rather than the client-side fan-out originally planned. -- **Discovery surface** ([#40](https://github.com/hotdata-dev/hotdata-langchain/issues/40)) — part 1 **shipped**: `hotdata_describe_tables` reports each +- ~~**Discovery surface**~~ ([#40](https://github.com/hotdata-dev/hotdata-langchain/issues/40)) — **done**, both parts. `hotdata_describe_tables` reports each column's `searchable_by`, read from `IndexesApi.list_indexes`, and hides the vector columns a - provider-backed index generated. No engine change was needed. Still open in that issue: - making multi-database tool sets distinguishable, and the un-pinning of the search corpus that - this now makes safe. + provider-backed index generated; no engine change was needed. `make_hotdata_tools` takes + `tool_name_suffix=` so two tool sets no longer both register `hotdata_execute_sql`, and the + database-scoped descriptions name their database. What this unlocks is not itself done: the + search corpus is still pinned at construction, and un-pinning it is now safe because an agent + can read what is searchable rather than guess. - ~~**Tool-layer robustness**~~ ([#41](https://github.com/hotdata-dev/hotdata-langchain/issues/41)) — **done.** `with_error_feedback` and `engine_error_message` are package API, `make_hotdata_tools(handle_errors=True)` turns the wrapping on, and both the sync and async callables are wrapped — the async one is what LangChain actually calls in a diff --git a/hotdata_langchain/__init__.py b/hotdata_langchain/__init__.py index 6d51751..684daa7 100644 --- a/hotdata_langchain/__init__.py +++ b/hotdata_langchain/__init__.py @@ -76,6 +76,7 @@ execute_sql_json, make_hotdata_tools, result_rows_for_llm, + suffixed_tool_name, ) from hotdata_langchain.vectorstore import HotdataVectorStore @@ -138,6 +139,7 @@ "result_rows_for_llm", "search_nouns_by_column", "semantic_search_json", + "suffixed_tool_name", "vector_distance_sql", "vector_search_sql", "with_error_feedback", diff --git a/hotdata_langchain/databases.py b/hotdata_langchain/databases.py index 2da84e0..b0bc9f0 100644 --- a/hotdata_langchain/databases.py +++ b/hotdata_langchain/databases.py @@ -84,6 +84,30 @@ def query_scope(database: ManagedDatabase | None) -> ManagedDatabase | None: ) +def scoped_description(description: str, label: str | None) -> str: + """Return ``description`` led by which database the tool works on. + + Two tool sets built over different databases put two identically-worded tools in one + prompt, and a model has nothing to choose between them on. The scope leads rather than + trails because it is the one fact that separates them. + + Returns ``description`` unchanged when there is no label, so a single-database tool set + carries no sentence about a distinction that does not exist. + """ + if not label: + return description + return f"Works on the '{label}' database. {description}" + + +def database_label(database: ManagedDatabase | None) -> str | None: + """Return the display name of ``database``, or ``None`` when it has none. + + The id is deliberately not a fallback. It carries no meaning a model can plan against, + and presenting one as a name invites passing it where a name is expected. + """ + return database.description if database is not None else None + + def query_catalogs(client: HotdataClient, database: ManagedDatabase) -> list[str]: """Return the catalogs that hold tables inside ``database``'s query scope. diff --git a/hotdata_langchain/schema.py b/hotdata_langchain/schema.py index 35d4181..eb2c64e 100644 --- a/hotdata_langchain/schema.py +++ b/hotdata_langchain/schema.py @@ -10,7 +10,11 @@ from langchain_core.tools import StructuredTool from hotdata_langchain._sql import quote_identifier, validate_identifier -from hotdata_langchain.databases import query_scope, resolve_database_by_id +from hotdata_langchain.databases import ( + query_scope, + resolve_database_by_id, + scoped_description, +) from hotdata_langchain.indexes import ( SearchIndex, generated_vector_columns, @@ -420,6 +424,7 @@ def make_hotdata_describe_tables_tool( column_stats: bool = True, search_capabilities: bool = True, catalogs: Sequence[str] | None = None, + label: str | None = None, ) -> StructuredTool: """Return a LangChain tool that reports the scoped database's tables and columns. @@ -435,6 +440,9 @@ def make_hotdata_describe_tables_tool( at the cost of one control-plane call per described table. Turn it off to describe a table without asking what is indexed on it. + ``label`` names the database in the description, so two tool sets over different + databases are distinguishable in one prompt. + Fails fast on a non-positive ``max_columns`` rather than at first invocation. """ if max_columns < 1: @@ -461,11 +469,14 @@ def hotdata_describe_tables(table: str | None = None) -> str: return StructuredTool.from_function( func=hotdata_describe_tables, name=name, - description=description - or default_describe_description( - column_stats=column_stats, - search_capabilities=search_capabilities, - catalogs=catalogs, + description=scoped_description( + description + or default_describe_description( + column_stats=column_stats, + search_capabilities=search_capabilities, + catalogs=catalogs, + ), + label, ), parse_docstring=True, ) diff --git a/hotdata_langchain/search.py b/hotdata_langchain/search.py index 579dcf7..59e48cd 100644 --- a/hotdata_langchain/search.py +++ b/hotdata_langchain/search.py @@ -19,7 +19,11 @@ validate_identifier, vector_literal, ) -from hotdata_langchain.databases import query_scope, resolve_database_by_id +from hotdata_langchain.databases import ( + query_scope, + resolve_database_by_id, + scoped_description, +) from hotdata_langchain.indexes import ( SEMANTIC, TEXT, @@ -1078,6 +1082,7 @@ def make_hotdata_search_tool( embedding: Embeddings | None = None, semantic_column: str | None = None, route: SearchRoute | None = None, + label: str | None = None, ) -> StructuredTool: """Return a LangChain tool that searches one indexed column, by text or by meaning. @@ -1119,6 +1124,9 @@ def make_hotdata_search_tool( does not yet take the table and column from the agent. It supplies only ``query`` and an optional ``k``. + ``label`` names the database in the description, so two tool sets over different + databases are distinguishable in one prompt. + ``database_id`` scopes the search to one instant database, by id and never by name; it is resolved once here. Pass an already-resolved ``ManagedDatabase`` to skip the lookup. @@ -1246,9 +1254,12 @@ def hotdata_search_text(query: str, k: int | None = None) -> str: return StructuredTool.from_function( func=hotdata_search_text, name=name or DEFAULT_SEARCH_TOOL_NAME, - description=description - or default_search_description( - table, column, columns=projection, max_k=max_rows, hybrid=fusion is not None + description=scoped_description( + description + or default_search_description( + table, column, columns=projection, max_k=max_rows, hybrid=fusion is not None + ), + label, ), parse_docstring=True, ) @@ -1294,9 +1305,12 @@ def hotdata_search_semantic(query: str, k: int | None = None) -> str: return StructuredTool.from_function( func=hotdata_search_semantic, name=name or DEFAULT_SEMANTIC_TOOL_NAME, - description=description - or default_semantic_description( - table, column, columns=projection, max_k=max_rows, composable=route.composable + description=scoped_description( + description + or default_semantic_description( + table, column, columns=projection, max_k=max_rows, composable=route.composable + ), + label, ), parse_docstring=True, ) diff --git a/hotdata_langchain/tools.py b/hotdata_langchain/tools.py index 845fe51..d7c0e4d 100644 --- a/hotdata_langchain/tools.py +++ b/hotdata_langchain/tools.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import re from collections.abc import Sequence from typing import Any @@ -13,6 +14,7 @@ from hotdata_langchain._sql import format_pattern_warnings from hotdata_langchain.databases import ( create_managed_database, + database_label, list_managed_databases_json, load_managed_table, load_result_summary, @@ -20,6 +22,7 @@ query_catalogs, query_scope, resolve_database_by_id, + scoped_description, ) from hotdata_langchain.errors import HotdataToolError, engine_error_message, with_error_feedback from hotdata_langchain.results import result_json @@ -38,6 +41,11 @@ resolve_search_route, ) +#: What a tool name may contain, and how long it may be. Tool-calling APIs validate the +#: name, and 64 is the shortest limit among the providers this package is used with. +TOOL_NAME_PATTERN = re.compile(r"[A-Za-z0-9_-]+") +MAX_TOOL_NAME_LENGTH = 64 + DEFAULT_SQL_TOOL_NAME = "hotdata_execute_sql" DEFAULT_LIST_DATABASES_TOOL_NAME = "hotdata_list_managed_databases" DEFAULT_CREATE_DATABASE_TOOL_NAME = "hotdata_create_managed_database" @@ -245,6 +253,32 @@ def sql_tool_description( ) +def suffixed_tool_name(name: str, suffix: str | None) -> str: + """Return ``name`` with ``suffix`` appended after an underscore. + + Registering two tool sets over different databases puts two tools called + ``hotdata_execute_sql`` in one prompt, and a model cannot address either. The suffix + is what separates them, so it is validated here rather than at the point a provider + rejects the call: it must be a bare token of letters, digits, underscores or hyphens, + and the result must fit the 64-character limit. + + Returns ``name`` unchanged when ``suffix`` is ``None``. + """ + if suffix is None: + return name + if not TOOL_NAME_PATTERN.fullmatch(suffix): + raise ValueError( + f"tool name suffix must be letters, digits, underscores or hyphens, got {suffix!r}" + ) + combined = f"{name}_{suffix}" + if len(combined) > MAX_TOOL_NAME_LENGTH: + raise ValueError( + f"{combined!r} is {len(combined)} characters; tool names are capped at " + f"{MAX_TOOL_NAME_LENGTH}, so {suffix!r} is too long a suffix" + ) + return combined + + def result_rows_for_llm(result: QueryResult, *, max_rows: int = 20) -> list[dict[str, Any]]: return result.to_records(max_rows=max_rows) @@ -309,6 +343,8 @@ def make_hotdata_tools( handle_errors: bool = False, allow_private_hosts: bool = False, catalog: str | None = None, + tool_name_suffix: str | None = None, + label: str | None = None, ) -> list[StructuredTool]: """Return LangChain tools for SQL and instant database workflows. @@ -329,6 +365,16 @@ def make_hotdata_tools( can be searched by, which is the one fact about a table that SQL cannot answer; it costs one control-plane call per table described. + ``tool_name_suffix`` appends a token to every tool name in the set, so two sets built + over different databases do not both register ``hotdata_execute_sql`` — a name a model + cannot address twice. An explicit ``search_tool_name`` is used exactly as given, since + naming that tool is already the caller's own decision. + + ``label`` names the database at the front of every description that is scoped to it — + the SQL, schema and search tools, but not the instant-database tools, which act on the + workspace rather than on one database. It defaults to the resolved database's name, + and a database with no name gets no sentence rather than one naming its id. + ``management_tools`` (on by default) adds the three tools that work on instant databases themselves — listing, creating and loading. Turn it off for an agent that reads one fixed database, where they are surface the model can only misuse. The flag @@ -488,23 +534,33 @@ def hotdata_load_managed_table( ) # The name reaches the model too, so it follows the route: a semantic search called # "search_text" would tell the model it matches wording, which is what it does not do. - resolved_search_name = search_tool_name or ( + resolved_search_name = search_tool_name or suffixed_tool_name( DEFAULT_SEMANTIC_TOOL_NAME if search_route is not None and search_route.semantic - else DEFAULT_SEARCH_TOOL_NAME + else DEFAULT_SEARCH_TOOL_NAME, + tool_name_suffix, ) + sql_name = suffixed_tool_name(DEFAULT_SQL_TOOL_NAME, tool_name_suffix) + describe_name = suffixed_tool_name(DEFAULT_DESCRIBE_TOOL_NAME, tool_name_suffix) + list_name = suffixed_tool_name(DEFAULT_LIST_DATABASES_TOOL_NAME, tool_name_suffix) + create_name = suffixed_tool_name(DEFAULT_CREATE_DATABASE_TOOL_NAME, tool_name_suffix) + load_name = suffixed_tool_name(DEFAULT_LOAD_TABLE_TOOL_NAME, tool_name_suffix) + scope_label = label if label is not None else database_label(database) tools = [ StructuredTool.from_function( func=hotdata_execute_sql, - name=DEFAULT_SQL_TOOL_NAME, - description=sql_tool_description( - resolved_search_name if has_search else None, - DEFAULT_DESCRIBE_TOOL_NAME if describe_tables else None, - search_table=search_table if has_search else None, - search_column=search_column if has_search else None, - search_route=search_route, - catalogs=catalogs, - max_rows=max_rows, + name=sql_name, + description=scoped_description( + sql_tool_description( + resolved_search_name if has_search else None, + describe_name if describe_tables else None, + search_table=search_table if has_search else None, + search_column=search_column if has_search else None, + search_route=search_route, + catalogs=catalogs, + max_rows=max_rows, + ), + scope_label, ), parse_docstring=True, ), @@ -513,7 +569,7 @@ def hotdata_load_managed_table( management = [ StructuredTool.from_function( func=hotdata_list_managed_databases, - name=DEFAULT_LIST_DATABASES_TOOL_NAME, + name=list_name, description=( "List the instant databases in this workspace. Returns each database's " "'id' and its human-readable 'name'. Names are display labels and are " @@ -524,7 +580,7 @@ def hotdata_load_managed_table( ), StructuredTool.from_function( func=hotdata_create_managed_database, - name=DEFAULT_CREATE_DATABASE_TOOL_NAME, + name=create_name, description=( "Create an instant database to hold tables you load. 'name' is a display " "label only and is not an identifier; the response carries the 'id', which " @@ -536,13 +592,13 @@ def hotdata_load_managed_table( ), StructuredTool.from_function( func=hotdata_load_managed_table, - name=DEFAULT_LOAD_TABLE_TOOL_NAME, + name=load_name, description=( "Load a parquet file into a table that was declared on an instant " "database, replacing whatever the table held. 'file' is either a path on " "the local filesystem or an http:// or https:// URL, which is downloaded " f"and uploaded for you{url_rule}. 'database_id' must be a database id returned by " - "hotdata_list_managed_databases or hotdata_create_managed_database — call " + f"{list_name} or {create_name} — call " "one of those first if you do not have an id. A database name is rejected: " "names are not unique, and this load overwrites the table, so the wrong " "target would destroy data. Only parquet is accepted, not CSV or JSON." @@ -559,9 +615,11 @@ def hotdata_load_managed_table( make_hotdata_describe_tables_tool( client, database_id=database, + name=describe_name, column_stats=describe_column_stats, search_capabilities=describe_search_capabilities, catalogs=catalogs, + label=scope_label, ) ) @@ -580,6 +638,7 @@ def hotdata_load_managed_table( database_id=database, embedding=search_embedding, route=search_route, + label=scope_label, ) ) diff --git a/tests/test_descriptions.py b/tests/test_descriptions.py index 8358937..7382964 100644 --- a/tests/test_descriptions.py +++ b/tests/test_descriptions.py @@ -336,6 +336,33 @@ def _tool_builds() -> list[tuple[str, dict[str, object]]]: "semantic search", {"search_table": TABLE, "search_column": COLUMN, "search_strategy": "semantic"}, ), + ( + "suffixed names", + { + "management_tools": True, + "search_table": TABLE, + "search_column": COLUMN, + "tool_name_suffix": "sales", + }, + ), + ( + "suffix with a digit", + { + "management_tools": True, + "search_table": TABLE, + "search_column": COLUMN, + "tool_name_suffix": "f1", + }, + ), + ( + "suffixed semantic search", + { + "search_table": TABLE, + "search_column": COLUMN, + "search_strategy": "semantic", + "tool_name_suffix": "sales", + }, + ), ] @@ -355,7 +382,7 @@ def _built(name: str, kwargs: dict[str, object]) -> list[StructuredTool]: the answer in. """ client = MagicMock() - if name != "semantic search": + if "semantic" not in name: return make_hotdata_tools(client, **kwargs) # type: ignore[arg-type] database = ManagedDatabase( @@ -416,7 +443,10 @@ def test_no_description_points_at_a_tool_that_is_not_registered( dangling = { (tool.name, ref) for tool in tools - for ref in re.findall(r"hotdata_[a-z_]+", _model_facing(tool)) + # Digits and hyphens are legal in a tool name, and a name-suffixed set puts them + # there: `[a-z_]+` truncated `hotdata_describe_tables_f1` to `..._f` and reported + # a correct reference as dangling. + for ref in re.findall(r"hotdata_[a-z0-9_-]+", _model_facing(tool)) if ref not in registered } assert not dangling, f"[{label}] descriptions name unregistered tools: {sorted(dangling)}" diff --git a/tests/test_tools.py b/tests/test_tools.py index 8d382ee..91dcfdb 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -473,3 +473,108 @@ def test_the_format_hint_survives_the_error_feedback_wrapper(mock_client): tools[DEFAULT_SQL_TOOL_NAME].invoke({"sql": "SELECT to_char(d, 'YYYY-MM-DD') FROM t"}) ) assert "Write '%Y-%m-%d' instead." in payload["error"] + + +# --- Telling two tool sets apart -------------------------------------------------- +# +# One client can query many databases, so registering two tool sets in one agent is a +# supported shape. Without a suffix both sets register `hotdata_execute_sql`, and the +# model is handed two tools it cannot address or choose between. + + +SALES = ManagedDatabase(id="dbid1", description="sales", default_connection_id="conn1") +SUPPORT = ManagedDatabase(id="dbid2", description="support", default_connection_id="conn2") + + +def _names(client: MagicMock, **kwargs: object) -> list[str]: + return [t.name for t in make_hotdata_tools(client, **kwargs)] # type: ignore[arg-type] + + +def test_two_tool_sets_collide_without_a_suffix(mock_client: MagicMock) -> None: + """The defect this exists to fix, pinned so the fix cannot be mistaken for unnecessary.""" + both = _names(mock_client, database_id=SALES) + _names(mock_client, database_id=SUPPORT) + assert len(set(both)) < len(both) + + +def test_a_suffix_makes_every_name_in_the_set_distinct(mock_client: MagicMock) -> None: + both = _names(mock_client, database_id=SALES, tool_name_suffix="sales") + _names( + mock_client, database_id=SUPPORT, tool_name_suffix="support" + ) + assert len(set(both)) == len(both) + assert "hotdata_execute_sql_sales" in both + assert "hotdata_execute_sql_support" in both + + +def test_an_explicit_search_tool_name_is_used_as_given(mock_client: MagicMock) -> None: + """Naming that tool is already the caller's decision, so the suffix does not second-guess it.""" + names = _names( + mock_client, + database_id=SALES, + search_table="default.public.listings", + search_column="description", + search_tool_name="search_sales", + tool_name_suffix="sales", + ) + assert "search_sales" in names + assert "hotdata_search_text_sales" not in names + + +@pytest.mark.parametrize( + "suffix", + [ + pytest.param("has a space", id="space"), + pytest.param("dots.not.allowed", id="dot"), + pytest.param("x" * 60, id="too-long"), + ], +) +def test_a_suffix_that_no_provider_would_accept_fails_at_build_time( + mock_client: MagicMock, suffix: str +) -> None: + """Rejected here rather than at the provider, which sees it only on the first call.""" + with pytest.raises(ValueError): + make_hotdata_tools(mock_client, database_id=SALES, tool_name_suffix=suffix) + + +def test_the_database_scoped_tools_name_their_database(mock_client: MagicMock) -> None: + tools = {t.name: t for t in make_hotdata_tools(mock_client, database_id=SALES)} + for name in ("hotdata_execute_sql", "hotdata_describe_tables"): + assert (tools[name].description or "").startswith("Works on the 'sales' database.") + + +def test_the_workspace_tools_do_not_claim_a_database(mock_client: MagicMock) -> None: + """Listing, creating and loading act on the workspace, so naming one would be false.""" + tools = {t.name: t for t in make_hotdata_tools(mock_client, database_id=SALES)} + for name in ( + "hotdata_list_managed_databases", + "hotdata_create_managed_database", + "hotdata_load_managed_table", + ): + assert "Works on the" not in (tools[name].description or "") + + +def test_the_label_defaults_to_the_database_name_and_can_be_overridden( + mock_client: MagicMock, +) -> None: + tools = { + t.name: t + for t in make_hotdata_tools(mock_client, database_id=SALES, label="EU support desk") + } + assert (tools["hotdata_execute_sql"].description or "").startswith( + "Works on the 'EU support desk' database." + ) + + +def test_a_database_with_no_name_gets_no_sentence_rather_than_one_naming_its_id( + mock_client: MagicMock, +) -> None: + """An id is not a name; presenting one as a name invites passing it where a name goes.""" + unnamed = ManagedDatabase(id="dbid3", description=None, default_connection_id="conn3") + tools = {t.name: t for t in make_hotdata_tools(mock_client, database_id=unnamed)} + described = tools["hotdata_execute_sql"].description or "" + assert "Works on the" not in described + assert "dbid3" not in described + + +def test_an_unscoped_tool_set_names_no_database(mock_client: MagicMock) -> None: + tools = {t.name: t for t in make_hotdata_tools(mock_client)} + assert "Works on the" not in (tools["hotdata_execute_sql"].description or "") From 59a2beb9880596f2489637db24c7c44170a05d1b Mon Sep 17 00:00:00 2001 From: Rohan Dsouza Date: Wed, 26 Aug 2026 17:46:55 +0530 Subject: [PATCH 3/3] fix: hide a generated vector column while its index is still building MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the three review nits on #87. `_table_indexes` now lists unready indexes too, and the caller filters differently for its two decisions. Whether a column is searchable *yet* depends on the index having finished; whether a column was generated by an index does not. With `ready_only=True` a provider-backed index still building was dropped before the filter ran, so `content_embedding` was described to the model as an ordinary List(Float32) column — the outcome the filter exists to prevent, in the window where the index is not usable anyway. Pinned by a test that fails against the old shape. The dangling-reference guard's pattern is widened to everything TOOL_NAME_PATTERN permits. `[a-z0-9_-]+` stops at an uppercase letter, so `tool_name_suffix="Sales"` — a legal build — would have had `hotdata_execute_sql_Sales` truncated to the prefix and reported as dangling. Same defect as the digit case, one character class away. An uppercase build joins the matrix, and the narrower pattern fails it. The management tools' names are resolved inside the branch that registers them, so a suffix too long for `hotdata_create_managed_database` — the longest base name — no longer fails a `management_tools=False` build by naming a tool the caller excluded. The three tools are also no longer constructed and discarded when the flag is off. Re-verified live: listing_corpus and listings_semantic annotate and filter as before. --- hotdata_langchain/schema.py | 16 +++++-- hotdata_langchain/tools.py | 92 +++++++++++++++++++------------------ tests/test_descriptions.py | 18 ++++++-- tests/test_schema.py | 33 +++++++++++++ tests/test_tools.py | 16 +++++++ 5 files changed, 123 insertions(+), 52 deletions(-) diff --git a/hotdata_langchain/schema.py b/hotdata_langchain/schema.py index eb2c64e..b80c372 100644 --- a/hotdata_langchain/schema.py +++ b/hotdata_langchain/schema.py @@ -138,16 +138,24 @@ def _table_indexes( name: str, database: ManagedDatabase | None, ) -> list[SearchIndex]: - """Return the search indexes on one table, or none when the scope cannot answer. + """Return every search index on one table, ready or not, or none when unscoped. Indexes belong to a connection, so an unscoped call has nothing to ask. Schema and table come from the rows ``information_schema`` returned rather than from re-parsing the reference, so a name this package would not have parsed still resolves. + + Unready indexes are kept because the caller makes two different decisions from this + list, and only one of them turns on readiness. Whether a column is searchable *yet* + does; whether a column was generated by an index does not. Filtering here would name + no capability and also stop hiding the vector column, which is the worse of the two + failures — it puts a float list in front of the model as ordinary data. """ if database is None: logger.debug("no database scope for %s.%s; not reporting searchable columns", schema, name) return [] - return list_search_indexes(client, table=name, schema=schema, database=database) + return list_search_indexes( + client, table=name, schema=schema, database=database, ready_only=False + ) def _column_stats( @@ -317,13 +325,15 @@ def describe_tables_json( if search_capabilities else [] ) + # Generated-ness does not depend on the index having finished building; being + # searchable does. Same listing, two filters. generated = set(generated_vector_columns(indexes)) records = [r for r in records if str(r["column_name"]) not in generated] names = [str(r["column_name"]) for r in records] columns: list[dict[str, object]] = [ {"name": name, "type": r["data_type"]} for name, r in zip(names, records, strict=True) ] - searchable = search_nouns_by_column(indexes) + searchable = search_nouns_by_column([index for index in indexes if index.ready]) annotated = False for entry in columns: nouns = searchable.get(str(entry["name"])) diff --git a/hotdata_langchain/tools.py b/hotdata_langchain/tools.py index d7c0e4d..5bf7863 100644 --- a/hotdata_langchain/tools.py +++ b/hotdata_langchain/tools.py @@ -542,9 +542,6 @@ def hotdata_load_managed_table( ) sql_name = suffixed_tool_name(DEFAULT_SQL_TOOL_NAME, tool_name_suffix) describe_name = suffixed_tool_name(DEFAULT_DESCRIBE_TOOL_NAME, tool_name_suffix) - list_name = suffixed_tool_name(DEFAULT_LIST_DATABASES_TOOL_NAME, tool_name_suffix) - create_name = suffixed_tool_name(DEFAULT_CREATE_DATABASE_TOOL_NAME, tool_name_suffix) - load_name = suffixed_tool_name(DEFAULT_LOAD_TABLE_TOOL_NAME, tool_name_suffix) scope_label = label if label is not None else database_label(database) tools = [ StructuredTool.from_function( @@ -566,49 +563,54 @@ def hotdata_load_managed_table( ), ] - management = [ - StructuredTool.from_function( - func=hotdata_list_managed_databases, - name=list_name, - description=( - "List the instant databases in this workspace. Returns each database's " - "'id' and its human-readable 'name'. Names are display labels and are " - "not unique — pass the 'id' to other tools, never the name. " - "An id cannot be guessed or built from a name; it only comes from here or " - "from creating a database." - ), - ), - StructuredTool.from_function( - func=hotdata_create_managed_database, - name=create_name, - description=( - "Create an instant database to hold tables you load. 'name' is a display " - "label only and is not an identifier; the response carries the 'id', which " - "is what every other tool needs — keep it. Declare the tables you intend " - "to load up front as a comma- or newline-separated list, so data loads " - "straight into them." - ), - parse_docstring=True, - ), - StructuredTool.from_function( - func=hotdata_load_managed_table, - name=load_name, - description=( - "Load a parquet file into a table that was declared on an instant " - "database, replacing whatever the table held. 'file' is either a path on " - "the local filesystem or an http:// or https:// URL, which is downloaded " - f"and uploaded for you{url_rule}. 'database_id' must be a database id returned by " - f"{list_name} or {create_name} — call " - "one of those first if you do not have an id. A database name is rejected: " - "names are not unique, and this load overwrites the table, so the wrong " - "target would destroy data. Only parquet is accepted, not CSV or JSON." - ), - parse_docstring=True, - ), - ] - if management_tools: - tools.extend(management) + # Names resolved here rather than above, so a suffix too long for one of these + # is reported against a tool the caller actually asked for. + list_name = suffixed_tool_name(DEFAULT_LIST_DATABASES_TOOL_NAME, tool_name_suffix) + create_name = suffixed_tool_name(DEFAULT_CREATE_DATABASE_TOOL_NAME, tool_name_suffix) + load_name = suffixed_tool_name(DEFAULT_LOAD_TABLE_TOOL_NAME, tool_name_suffix) + tools.extend( + [ + StructuredTool.from_function( + func=hotdata_list_managed_databases, + name=list_name, + description=( + "List the instant databases in this workspace. Returns each database's " + "'id' and its human-readable 'name'. Names are display labels and are " + "not unique — pass the 'id' to other tools, never the name. " + "An id cannot be guessed or built from a name; it only comes from here or " + "from creating a database." + ), + ), + StructuredTool.from_function( + func=hotdata_create_managed_database, + name=create_name, + description=( + "Create an instant database to hold tables you load. 'name' is a display " + "label only and is not an identifier; the response carries the 'id', which " + "is what every other tool needs — keep it. Declare the tables you intend " + "to load up front as a comma- or newline-separated list, so data loads " + "straight into them." + ), + parse_docstring=True, + ), + StructuredTool.from_function( + func=hotdata_load_managed_table, + name=load_name, + description=( + "Load a parquet file into a table that was declared on an instant " + "database, replacing whatever the table held. 'file' is either a path on " + "the local filesystem or an http:// or https:// URL, which is downloaded " + f"and uploaded for you{url_rule}. 'database_id' must be a database id " + f"returned by {list_name} or {create_name} — call one of those first if " + "you do not have an id. A database name is rejected: " + "names are not unique, and this load overwrites the table, so the wrong " + "target would destroy data. Only parquet is accepted, not CSV or JSON." + ), + parse_docstring=True, + ), + ] + ) if describe_tables: tools.append( diff --git a/tests/test_descriptions.py b/tests/test_descriptions.py index 7382964..54ae114 100644 --- a/tests/test_descriptions.py +++ b/tests/test_descriptions.py @@ -354,6 +354,15 @@ def _tool_builds() -> list[tuple[str, dict[str, object]]]: "tool_name_suffix": "f1", }, ), + ( + "suffix with an uppercase letter", + { + "management_tools": True, + "search_table": TABLE, + "search_column": COLUMN, + "tool_name_suffix": "Sales", + }, + ), ( "suffixed semantic search", { @@ -443,10 +452,11 @@ def test_no_description_points_at_a_tool_that_is_not_registered( dangling = { (tool.name, ref) for tool in tools - # Digits and hyphens are legal in a tool name, and a name-suffixed set puts them - # there: `[a-z_]+` truncated `hotdata_describe_tables_f1` to `..._f` and reported - # a correct reference as dangling. - for ref in re.findall(r"hotdata_[a-z0-9_-]+", _model_facing(tool)) + # The class has to be everything TOOL_NAME_PATTERN permits. A narrower one + # truncates a suffixed name at the first character it excludes and reports the + # prefix as dangling, failing a correct build: `[a-z_]+` did that to + # `hotdata_describe_tables_f1`, and `[a-z0-9_-]+` would to `..._Sales`. + for ref in re.findall(r"hotdata_[A-Za-z0-9_-]+", _model_facing(tool)) if ref not in registered } assert not dangling, f"[{label}] descriptions name unregistered tools: {sorted(dangling)}" diff --git a/tests/test_schema.py b/tests/test_schema.py index a55a98a..52ff3dc 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -622,6 +622,39 @@ def test_an_index_still_building_is_not_reported_as_searchable(search_indexes: M assert "search" not in payload +def test_a_building_provider_index_still_hides_the_column_it_generated( + search_indexes: MagicMock, +) -> None: + """Two decisions come off one listing, and only one of them turns on readiness. + + A generated vector column is generated whether or not the index over it has finished + building. Dropping unready indexes before this filter would describe a float list to + the model as ordinary data — the outcome the filter exists to prevent — in the window + where the index is not usable anyway. + """ + columns = result( + ["table_schema", "table_name", "column_name", "data_type"], + [ + ["public", "listings", "description", "LargeUtf8"], + ["public", "listings", "description_embedding", "List(Float32)"], + ], + ) + payload, _ = _described( + search_indexes, + [ + vector_index( + columns=["description_embedding"], + source_column="description", + status="building", + ) + ], + columns=columns, + ) + assert [c["name"] for c in payload["columns"]] == ["description"] + assert all("searchable_by" not in c for c in payload["columns"]) + assert "search" not in payload + + def test_the_note_explaining_searchable_by_appears_only_when_something_carries_it( search_indexes: MagicMock, ) -> None: diff --git a/tests/test_tools.py b/tests/test_tools.py index 91dcfdb..e18220d 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -535,6 +535,22 @@ def test_a_suffix_that_no_provider_would_accept_fails_at_build_time( make_hotdata_tools(mock_client, database_id=SALES, tool_name_suffix=suffix) +def test_the_length_limit_is_checked_against_the_tools_actually_built( + mock_client: MagicMock, +) -> None: + """Otherwise the error names a tool the caller excluded, which reads as a bug. + + `hotdata_create_managed_database` is the longest base name in the set, so a suffix can + be too long for it and fine for everything else. + """ + suffix = "x" * 33 + names = _names(mock_client, database_id=SALES, management_tools=False, tool_name_suffix=suffix) + assert names and all(n.endswith(suffix) for n in names) + + with pytest.raises(ValueError, match="hotdata_create_managed_database"): + make_hotdata_tools(mock_client, database_id=SALES, tool_name_suffix=suffix) + + def test_the_database_scoped_tools_name_their_database(mock_client: MagicMock) -> None: tools = {t.name: t for t in make_hotdata_tools(mock_client, database_id=SALES)} for name in ("hotdata_execute_sql", "hotdata_describe_tables"):