feat(py): build_commons_tools() and the tool bodies - #310
Merged
Conversation
jat255
added
r
jat255
force-pushed
the
jat255/hsjq-tools
branch
from
September 7, 2026 20:57
a8612ef to
14dccfa
Compare
jat255
changed the base branch from
jat255/g9yd-agent-helpers
to
jat255/6fv1-prompt-data
September 7, 2026 20:57
The tools that return data all need the same three things, so they land before the tools do: rows rendered as a capped pipe table, a backend that can report a relation's columns when no warehouse listing already carries them, and a dictionary that merges those live columns with what it documents about them. `sqlglot_dialect()` comes out of the SQL guard's privates, because compiling a governed query needs the same backend-to-sqlglot mapping to quote an identifier or a literal for the right dialect.
One discovery surface over measures and governed definitions, ranked together so the model does not have to guess which kind holds its answer, plus the metrics-by-dimensions-with-filters query the pool's metrics are run through. Identifiers and literals are written by sqlglot for the source's own dialect rather than by hand, so an apostrophe in a predicate value cannot end the literal early, and a backend that spells identifiers with backticks gets those.
Each tool is registered only when the agent's composition earns it: search_pool once the pool holds something the prompt does not already show, call_measure with measures, call_metrics with a governed metric, search_catalog with a catalog too broad to list. search_context, describe_table, and run_sql come with any source. describe_table and run_sql share the first-touch tracker, so a table's dictionary entry is put in front of the model once however the model first reached it. The result is a list of chatlas tools rather than a mutated client, so the surface can be built and read before there is a conversation to attach it to. call_calculation is left out and call_metrics covers governed definitions alone: both of the missing halves read a warehouse's own semantic objects, which this package does not have yet.
tool-registration.json declares a composition and the tools it earns, plus the description each registered tool gives the model. Both are prompt surface: registration is what the model can reach, and a description is what it reads before choosing. Two things the file deliberately leaves out. Tool arguments, because a JSON Schema property and an ellmer type are different objects and normalizing one into the other would pin the plumbing rather than what the model reads. And the execution tool, because it is run_r in one package and run_python in the other and each is built by its own owner, so a runner drops both names before comparing. No case declares a warehouse's own semantic objects or exact trusted queries, so call_calculation and the semantic half of call_metrics never register in either package. That is the fixture staying inside what both implement while gcgj is deferred, and the file says so.
A refusal is an answer, so a relation the principal may not query is left out of a catalog search. A timeout or an unreadable failure is not an answer, and treating it as one reports a relation the agent does have as one it does not. pkg-r/R/catalog.R draws the line in the same place.
search_pool and call_metrics were registered but never run, and describe_table over a pins board leans on the sample query having loaded the pin before the column read that follows it.
A board source reads a pin when a query names it, and that recovery lives on DataSource.query(). A measure is handed the connection itself, so nothing there fires it: a board-backed measure failed on its first call with a catalog error for a relation that had not been written yet. call_measure now ensures each injected source has read its pins before invoking the measure, which is the step pkg-r/R/tools.R takes through source_ensure_all() for the same reason. DataSource.ensure_loaded() is the eager entry point, driving the same _load_pins() the query path uses rather than a second copy of it.
Five rows of values tell a model less about a column than its range, its
missing count, and the values it actually takes, so describe_table now
shows the same column-by-column summary the R agent gets from ellmer's
df_schema(): the same header line, the same facts per kind of column, in
the same order, with the same punctuation. A table whose sample came back
empty is still described column by column, because the schema fixes the
columns rather than the rows.
The type token in front of those facts is the one word the two packages do
not share. R names R's types because it holds an R vector; this names
Python's, because a row here is a mapping of Python values and calling an
int an integer would describe a vector nobody has. The mapping is written
out at the top of _sample_summary.py.
sample-summary.json pins the wording and leaves the token to each package,
so a case carries a {type} placeholder and the word each side puts there.
The one shape it cannot pin is an all-null column whose R type still
carries facts of its own, and it says so.
Ranges are rendered to four significant digits the way R's format() does,
fixed unless scientific is shorter, since the wording around a range is
shared and the numbers inside it have to agree. Checked against 330 values
read off R.
Two defects in the numeric half of the sample summary. Every value went through float before it was rendered, so a 2147483647 id came back as 2.147e+09 where R writes it out, and a DECIMAL column was rounded to a double's precision before anything asked it to be. A whole number is now written in full however wide it is, and a Decimal is formatted as itself, with its exponent padded so the same magnitude reads alike from a DECIMAL and a DOUBLE column. A NaN was an ordinary value, so min() returned it or not depending on which row the query happened to return first, and the same table could summarize two ways. R counts a NaN as NA and leaves it out of the range; so does this now. Both are shared behaviour, so both earn a case in sample-summary.json, which needed a way to spell a not-a-number that JSON has no literal for.
math.isnan() refuses to convert a signalling NaN to a float and raises, so a DECIMAL column holding one would have thrown out of describe_table instead of counting it as missing. Decimal answers for itself. Also guards a column mixing an aware timestamp with a naive one, which Python will not order and which would otherwise raise from the range: no driver returns both from one column, so it is reported as mixed. The fixture's note on how it spells a not-a-number now stands on its own sentence rather than pointing back at the paragraph before it.
Python calls a timestamp naive when utcoffset() answers None, which a tzinfo can do while still being attached and still naming a zone. Deciding on tzinfo alone let such a value count as aware, so it could be compared against a genuinely aware one and raise from the range it was meant to guard, and it could put a zone in front of facts about a naive column. One offset answer for the whole column now decides both.
jat255
force-pushed
the
jat255/hsjq-tools
branch
from
September 8, 2026 04:40
14dccfa to
5843b34
Compare
|
Preview deployed to Connect ( Deployed from commit acba79f. |
|
Preview deployed to Connect ( Deployed from commit acba79f. |
- A null in a result table reads NA, a boolean TRUE or FALSE, and a float keeps seven significant digits, so the model meets one spelling per idea across the tools and both packages. The three knitr::kable behaviors deliberately not matched are documented in _rows.py. - A measure that returns a ContentToolResult is returned untouched, and a list result past twenty items is capped and reported rather than printed in full. - Cover the warehouse describe dispatch and its column caching, and the measure-argument parsing branches (a mapping passed through, a non-object rejected). - Stop claiming the pool's sibling scoping matches R; it is deliberate.
jat255
removed
the
needs-manual-review
jat255
marked this pull request as ready for review
September 8, 2026 05:16
simonpcouch
approved these changes
Sep 8, 2026
|
Cleaned up 2 preview bundle(s) on https://dogfood.team.pct.posit.it: 368842, 368844 |
|
Cleaned up 2 preview bundle(s) on https://connect.staging.pct.posit.it: 2686, 2688 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Registers the tools an agent can reach and implements their bodies, which return plain text.
pvrd(#311) builds aToolContext, callsbuild_commons_tools(), and registers each tool it returns on its chat client.Implementation
search_pool,call_measure,call_metrics,search_catalog,search_context,describe_table,run_sql. R's eighth,call_calculation, and the semantic-model half ofcall_metricsboth read a warehouse's own semantic objects, which is deferred post-conf.run_pythonbelongs to M6 and will be added later.describe_tableandrun_sql: a table's dictionary entry goes in front of the model the first time either tool reaches it, keyed on the resolved source label so a table reachable under two names is not delivered twice._sample_summary.py), as R does, rather than printed. Ranges render to four significant digits the way R'sformat()does, because the wording around a range is shared and the numbers inside it have to agree._rows.py): a null readsNA, a booleanTRUE/FALSE, a float seven significant digits. Threeknitr::kablebehaviors are deliberately not matched —No rows.instead of a header-only empty table, line breaks folded to spaces,\|rather than|— and the module docstring records why. A measure returning aContentToolResultis returned untouched; a list past twenty items is capped and reported.call_measureloads a board source's pins before invoking a measure, as R does: the measure holds a connection, not theDataSource, so nothing else would trigger the read.Tests
tool-registration.jsonpins which tools each composition registers and what each tells the model. Arguments are not pinned: a JSON Schema property and an ellmer type are different objects, and normalizing one into the other would pin plumbing. Two asymmetries are documented in the fixture — no case declares warehouse semantic objects, so those tools register in neither language, and each runner drops the per-language execution-tool names before comparing.sample-summary.jsonpins the summary's wording but not the type token (R names R's types, Python names Python's), via a{type}placeholder per column. One shape is deliberately left out — an all-null string/boolean column — and the fixture says so.r-format-signif.jsonholds 330 R-generated values, since R is not available to the Python suite.Verified:
ruff check,pyrefly check src tests,pytest(1235 passing), and both R runners (39 and 16 passing).R changes
@simonpcouch
No R source or behavior changed. R gains two fixture runners — test-tool-registration.R and test-sample-summary.R — and the synced fixture copies. Both runners read R's existing behavior and compare it against the fixtures; no hand-written R tests were deleted.
Evidence: both R test files pass under the framework R with
pkgload::load_all(). The full R suite was not run.