feat: compact row encoding for run_query/preview_table (v0.31.0) - #45
Merged
Conversation
Spec for replacing dict-per-row JSON in run_query/preview_table with arrays-of-arrays aligned to the existing columns key, behind a create_tools(row_format=...) operator knob defaulting to compact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- compact mode must coerce rows with list(row): json.dumps sends a non-list/tuple row through default=str and serializes it as a string, where dict(zip(...)) tolerated any iterable - create_pydantic_ai_toolset validates row_format before returning its per-run factory, so the eager-validation guarantee holds on all paths - pin RowFormat's home and export, following the Principal precedent - qualify the zero-row columns claim as adapter-dependent - name the new test file, add a non-tuple-row test Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Five sequential TDD tasks: _render_rows helper, create_tools row_format param + run_query, preview_table + columns key, wrapper forwarding, docs and 0.31.0 release. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pure rendering helper for result rows, positional arrays or one dict per row. Not yet wired into any tool. Refs #44 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds create_tools(row_format=...) with eager validation. Compact is the default; row_format='records' restores the dict-per-row shape. Refs #44
Both modes now emit {schema, table, columns, rows}, so the two result
tools share one envelope and a zero-row preview still describes the
table's shape. Refs #44
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
All four wrappers accept and forward row_format; the pydantic-ai toolset validates it before returning its per-run factory so the fail-at-wiring guarantee holds on every path. Refs #44
Extract _validate_row_format() in factory.py so create_tools and create_pydantic_ai_toolset share one error message instead of two copies that could drift. Rewrite test_sdk_wrapper_accepts_row_format to actually call create_sdk_mcp_server and assert the ValueError surfaces, since the old version only checked the parameter's presence in the function signature and would not have caught a dropped row_format= forward. Addresses review findings I1 and I2 on Task 4 of issue #44. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closes the pydantic-ai row_format forwarding gap that the 810-test suite didn't catch (two new tests, each verified by deletion-based regression checks against pydantic_ai.py:131 and :266), corrects the "identical information" claim in README/CHANGELOG for duplicate column labels, strengthens the description-join and columns-before- rows tests, narrows three bare `# type: ignore`s to the codebase's `# ty: ignore[...]` form, drops the misleading underscore on factory.validate_row_format (the only private symbol crossing a src/ module boundary), and trims overstated/process-only CHANGELOG wording. No runtime behaviour changes; full suite green (812 passed) and prek clean.
Owner
Author
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
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
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
run_queryandpreview_tablerendered every result row as a JSON object repeating all column names. For a wide result that roughly doubles to triples the token cost of a tool result, for identical information — and because tool results persist in the message history and are re-sent on every subsequent model request, that overhead is paid repeatedly rather than once.rowsis now a list of positional arrays aligned to thecolumnskey thatrun_queryalready returned.Measured, realistic short column names:
Closes #44.
Design decisions
The knob is operator-facing, not a model-facing tool argument. Unlike Anthropic's
concise/detailedpattern, this drops no information, so the agent has no basis on which to choose — and aninput_schemafield would cost tokens on every request.create_tools(..., row_format="records")restores the previous rendering.Arrays-of-arrays, not TSV. TSV is ~9 points smaller but needs hand-written escaping and collapses
NULLinto the empty string. In a library whose purpose is trustworthy reported numbers, a rendering that cannot distinguish "no value" from "empty value" was rejected.preview_tablegainscolumnsin both renderings, so both result tools share one{columns, rows}envelope. This also closes a real gap: a zero-row preview previously returned{"rows": []}and told the agent nothing about the table's shape.Validation is eager, at
create_tools()wiring time rather than render time, because tool callables are async and their failures surface as agent-visible text.create_pydantic_ai_toolsetvalidates in its own body rather than deferring to its per-run factory, so the guarantee holds on every path._render_rows'slist(row)coercion is load-bearing.DatabaseAdapteris a@runtime_checkableProtocol, so a third-party adapter may return its driver's row type.dict(zip(...))tolerated that (it needs only iteration) butjson.dumpswould route it throughdefault=strand emit a string instead of an array.Compatibility
The default output shape of two tools changed. Anything parsing
rowsas a list of dicts must either read positionally (row[columns.index("col")]) or passrow_format="records". Values are unaffected —json.dumps(..., default=str)is unchanged.One nuance found during review and documented: the two renderings are not informationally identical when column labels collide.
SELECT t.id, u.idreturns['id', 'id'], andrecords'dict(zip(...))is last-value-wins, so a column silently vanishes.compactpreserves both — a second argument for the new default.Non-goals
No row cap or truncation (
require_limitandresult_check.max_rowsalready exist as opt-in contract rules; blocking beats silently truncating). No third format. No model-facing tool argument. No wrapper refactor beyond the one parameter.Testing
Full suite 812 passed;
prek run --all-files(ruff check, ruff format, ty) clean under the newly pinned hooks. Built TDD, red-first, across five reviewed tasks. Both pydantic-ai forwards and the SDK forward are regression-checked by deletion — removing the forward makes its test fail.🤖 Generated with Claude Code