Skip to content

Commit 9c1c340

Browse files
rustyconoverclaude
andcommitted
codegen: emit the inner request schemas; fix the fixture-server drift
The generators reached INFO_TYPES, EXTRA_RESPONSE_TYPES and each method's params/result, but never the sixteen inner REQUEST records — BindRequest, InitRequest, CatalogAttachRequest, the aggregate and table-buffering families, the scan-planning pair. Nothing reached them because a method's params schema is the outer envelope (`request: binary`) and the record rides inside as an opaque blob. So every peer that builds one of those records built it against nothing. That is not theoretical: emitting them found a wrong-order/short BindRequest and InitRequest in the C++ client, a `list<int32>` where the protocol says int64 plus three binary/large_binary mismatches in Go, and a BindRequest in TypeScript whose column count varied with whether the call was a COPY. REQUEST_TYPES now lives in _common.py so all five emitters share one list, and cpp/go/rust/ts join java in emitting it. Two type gaps surfaced immediately and are fixed: cpp and go had no large_string / large_binary / large_list mapping (InitRequest.pushdown_filters is large_binary), and ts gained largeUtf8/largeBinary on both Arrow backends. ts deliberately REFUSES large_list rather than narrowing it to a 32-bit list — arrow-js has no LargeList type, and silently emitting a `list` would make the TS worker reject a batch every other SDK accepts. Separately, vgi-fixture-http omitted NarrowBindWorker while vgi-fixture-worker registered it, so narrow_bind_mismatch.test — which guards a client SIGSEGV — had never run over HTTP. The two lists are now guarded by tests/test_fixture_catalog_parity.py, which parses both entry points rather than executing them (calling either starts a server). Also drops the [tool.uv.sources] path override for vgi-rpc: 0.43.0 is on PyPI now, which is what its comment said to wait for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 9afd066 commit 9c1c340

14 files changed

Lines changed: 339 additions & 161 deletions

‎pyproject.toml‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -258,13 +258,6 @@ members = ["packages/vgi-fixtures"]
258258

259259
[tool.uv.sources]
260260
# duckdb = { path = "/path/to/duckdb-python/dist/duckdb-*.whl" } # local engine-build override
261-
# Local co-development source, REQUIRED until vgi-rpc 0.43.0 is published: the
262-
# floor above asks for a release that is not on PyPI yet, and `uv run`/`uv sync`
263-
# do not merely fail to resolve — they DELETE .venv on the way out. Two separate
264-
# sessions lost their environment to this in one afternoon, and a restored venv
265-
# silently carries the pre-0.43.0 schemas, which no longer match this extension.
266-
# Drop this line once 0.43.0 ships. Mirrors vgi-rust's [patch.crates-io] stanza.
267-
vgi-rpc = { path = "../vgi-rpc-python", editable = true }
268261
vgi-fixtures = { workspace = true }
269262

270263
[dependency-groups]
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
# Copyright 2025, 2026 Query Farm LLC - https://query.farm
2+
3+
"""The two fixture-worker entry points must serve the same catalogs.
4+
5+
``vgi-fixture-worker`` (subprocess/launcher transport) and ``vgi-fixture-http``
6+
(HTTP transport) each assemble their own list of worker classes. The SAME
7+
``.test`` files run against both, so a catalog present in one list and absent
8+
from the other fails those files on one transport only — which reads as a
9+
transport-specific bug and is not one.
10+
11+
That is not hypothetical. ``NarrowBindWorker`` was added to the subprocess entry
12+
point and not the HTTP one, so ``narrow_bind_mismatch.test`` failed every HTTP
13+
run with ``No worker handles catalog 'narrow_bind'``. That test guards a client
14+
SIGSEGV — a worker whose bind ``output_schema`` is narrower than the table's
15+
advertised columns used to walk off the end of the batch in
16+
``ArrowTableFunction::ArrowToDuckDB`` — so it was the worst one to silently lose
17+
on a transport.
18+
19+
Both lists are built inside function bodies with local imports (deliberately:
20+
the fixture extras are optional, and a module-level import would make the whole
21+
package require them). So this reads the source rather than calling the
22+
functions, which would start a server.
23+
"""
24+
25+
from __future__ import annotations
26+
27+
import ast
28+
import inspect
29+
from pathlib import Path
30+
31+
import pytest
32+
33+
# Workers each entry point may legitimately add on its own. The writable
34+
# catalog is conditional on an optional extra in BOTH, and is appended outside
35+
# the literal list, so it never appears in either parsed set.
36+
_ALLOWED_ASYMMETRY: dict[str, str] = {}
37+
38+
39+
def _worker_class_names(module_path: Path, func_name: str) -> set[str]:
40+
"""Names in the `worker_classes = [...]` / `workers = [...]` literal.
41+
42+
Parsed rather than executed: calling either function starts a worker.
43+
"""
44+
tree = ast.parse(module_path.read_text())
45+
46+
target: ast.FunctionDef | None = None
47+
for node in ast.walk(tree):
48+
if isinstance(node, ast.FunctionDef) and node.name == func_name:
49+
target = node
50+
break
51+
assert target is not None, f"{module_path.name} has no function {func_name}()"
52+
53+
for node in ast.walk(target):
54+
if not isinstance(node, ast.AnnAssign | ast.Assign):
55+
continue
56+
targets = [node.target] if isinstance(node, ast.AnnAssign) else node.targets
57+
names = {t.id for t in targets if isinstance(t, ast.Name)}
58+
if not names & {"worker_classes", "workers"}:
59+
continue
60+
if not isinstance(node.value, ast.List):
61+
continue
62+
found = {e.id for e in node.value.elts if isinstance(e, ast.Name)}
63+
if found:
64+
return found
65+
66+
pytest.fail(
67+
f"{module_path.name}:{func_name}() no longer assigns a list literal of worker "
68+
"classes to `worker_classes` or `workers`. This guard now matches nothing — "
69+
"update the parser rather than deleting the test."
70+
)
71+
72+
73+
@pytest.fixture(scope="module")
74+
def _lists() -> tuple[set[str], set[str]]:
75+
import vgi._test_fixtures.http_server as http_server
76+
import vgi._test_fixtures.worker as subprocess_worker
77+
78+
sub = _worker_class_names(Path(inspect.getfile(subprocess_worker)), "main")
79+
http = _worker_class_names(Path(inspect.getfile(http_server)), "main")
80+
return sub, http
81+
82+
83+
def test_both_lists_are_non_empty(_lists: tuple[set[str], set[str]]) -> None:
84+
"""A parser that silently matches nothing would make this whole file vacuous."""
85+
sub, http = _lists
86+
assert len(sub) >= 2, f"subprocess entry point parsed as {sub} — parser is broken"
87+
assert len(http) >= 2, f"HTTP entry point parsed as {http} — parser is broken"
88+
89+
90+
def test_entry_points_serve_the_same_catalogs(_lists: tuple[set[str], set[str]]) -> None:
91+
"""Neither entry point may serve a catalog the other does not."""
92+
sub, http = _lists
93+
only_sub = sorted(sub - http - set(_ALLOWED_ASYMMETRY))
94+
only_http = sorted(http - sub - set(_ALLOWED_ASYMMETRY))
95+
assert not only_sub and not only_http, (
96+
"the fixture entry points serve different catalogs, so the shared .test files "
97+
"will fail on one transport only:\n"
98+
f" only in vgi-fixture-worker (subprocess): {only_sub}\n"
99+
f" only in vgi-fixture-http: {only_http}\n"
100+
"Add the missing worker to the other list, or record it in _ALLOWED_ASYMMETRY "
101+
"with the reason it is genuinely one-transport."
102+
)
103+
104+
105+
def test_allowed_asymmetry_stays_honest(_lists: tuple[set[str], set[str]]) -> None:
106+
"""An excuse for a worker neither list mentions reads as coverage that isn't there."""
107+
sub, http = _lists
108+
for name, reason in _ALLOWED_ASYMMETRY.items():
109+
assert name in sub or name in http, (
110+
f"_ALLOWED_ASYMMETRY excuses {name!r} ({reason}), but neither entry point "
111+
"mentions it — drop the entry."
112+
)

‎tests/test_generated_cpp_schemas.py‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import pyarrow as pa
2222
import pytest
2323

24+
from vgi.codegen._common import EXTRA_RESPONSE_TYPES, REQUEST_TYPES
2425
from vgi.codegen.cpp_schemas import _collect_schemas, emit
2526

2627

@@ -88,11 +89,19 @@ def _parse_type(expr: str) -> pa.DataType:
8889
"arrow::float64()": pa.float64(),
8990
"arrow::utf8()": pa.string(),
9091
"arrow::binary()": pa.binary(),
92+
"arrow::large_utf8()": pa.large_string(),
93+
"arrow::large_binary()": pa.large_binary(),
9194
"arrow::null()": pa.null(),
9295
}
9396
if expr in simple:
9497
return simple[expr]
9598

99+
if expr.startswith("arrow::large_list(") and expr.endswith(")"):
100+
inner = expr[len("arrow::large_list(") : -1]
101+
if inner.startswith("arrow::field("):
102+
return cast("pa.DataType", pa.large_list(_parse_field(inner)))
103+
return cast("pa.DataType", pa.large_list(_parse_type(inner)))
104+
96105
if expr.startswith("arrow::list(") and expr.endswith(")"):
97106
inner = expr[len("arrow::list(") : -1]
98107
return pa.list_(_parse_type(inner))
@@ -191,7 +200,12 @@ def test_checked_in_generated_hpp_matches_generator() -> None:
191200
pytest.skip(f"{path} not found; set VGI_GENERATED_HPP or check out vgi repo next to vgi-python")
192201

193202
actual = _parse_generated_hpp(path.read_text())
194-
expected = {es.name: es.schema for es in _collect_schemas()}
203+
expected = {
204+
es.name: es.schema
205+
for es in _collect_schemas(
206+
extra_response_types=(*EXTRA_RESPONSE_TYPES, *REQUEST_TYPES),
207+
)
208+
}
195209

196210
# Extra or missing factories.
197211
missing = set(expected) - set(actual)
@@ -214,7 +228,12 @@ def test_parser_roundtrip_self_test() -> None:
214228
buf = io.StringIO()
215229
emit(buf)
216230
parsed = _parse_generated_hpp(buf.getvalue())
217-
expected = {es.name: es.schema for es in _collect_schemas()}
231+
expected = {
232+
es.name: es.schema
233+
for es in _collect_schemas(
234+
extra_response_types=(*EXTRA_RESPONSE_TYPES, *REQUEST_TYPES),
235+
)
236+
}
218237
assert set(parsed) == set(expected), "parser missed a factory the generator emitted"
219238
for name, schema in expected.items():
220239
assert schema.equals(parsed[name], check_metadata=False), (

‎tests/test_generated_go_schemas.py‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import pyarrow as pa
2222
import pytest
2323

24-
from vgi.codegen._common import collect_schemas
24+
from vgi.codegen._common import EXTRA_RESPONSE_TYPES, REQUEST_TYPES, collect_schemas
2525
from vgi.codegen.go_schemas import emit
2626

2727

@@ -88,6 +88,8 @@ def _split_top_level_comma(expr: str) -> list[str]:
8888
"arrow.PrimitiveTypes.Float64": pa.float64(),
8989
"arrow.BinaryTypes.String": pa.string(),
9090
"arrow.BinaryTypes.Binary": pa.binary(),
91+
"arrow.BinaryTypes.LargeString": pa.large_string(),
92+
"arrow.BinaryTypes.LargeBinary": pa.large_binary(),
9193
}
9294

9395

@@ -104,6 +106,18 @@ def _parse_type(expr: str) -> pa.DataType:
104106
inner = expr[len("arrow.ListOfField(") : -1]
105107
return pa.list_(_parse_field(inner))
106108

109+
# `LargeListOf` must be tested BEFORE `ListOf`: the plain prefix check
110+
# below would never see it (different stem), but keeping the pair adjacent
111+
# is what stops a future `startswith("arrow.List")` shortcut from silently
112+
# narrowing a large list to a 32-bit one.
113+
if expr.startswith("arrow.LargeListOf(") and expr.endswith(")"):
114+
inner = expr[len("arrow.LargeListOf(") : -1]
115+
return pa.large_list(_parse_type(inner))
116+
117+
if expr.startswith("arrow.LargeListOfField(") and expr.endswith(")"):
118+
inner = expr[len("arrow.LargeListOfField(") : -1]
119+
return pa.large_list(_parse_field(inner))
120+
107121
if expr.startswith("arrow.MapOf(") and expr.endswith(")"):
108122
inside = expr[len("arrow.MapOf(") : -1]
109123
parts = _split_top_level_comma(inside)
@@ -184,7 +198,10 @@ def test_checked_in_generated_go_matches_generator() -> None:
184198
)
185199

186200
actual = _parse_generated_go(path.read_text())
187-
expected = {es.name: es.schema for es in collect_schemas()}
201+
expected = {
202+
es.name: es.schema
203+
for es in collect_schemas(extra_response_types=(*EXTRA_RESPONSE_TYPES, *REQUEST_TYPES))
204+
}
188205

189206
missing = set(expected) - set(actual)
190207
extra = set(actual) - set(expected)
@@ -206,7 +223,10 @@ def test_parser_roundtrip_self_test() -> None:
206223
buf = io.StringIO()
207224
emit(buf)
208225
parsed = _parse_generated_go(buf.getvalue())
209-
expected = {es.name: es.schema for es in collect_schemas()}
226+
expected = {
227+
es.name: es.schema
228+
for es in collect_schemas(extra_response_types=(*EXTRA_RESPONSE_TYPES, *REQUEST_TYPES))
229+
}
210230
assert set(parsed) == set(expected), "parser missed a factory the generator emitted"
211231
for name, schema in expected.items():
212232
assert schema.equals(parsed[name], check_metadata=False), (

‎tests/test_generated_rust_schemas.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
import pyarrow as pa
3030
import pytest
3131

32-
from vgi.codegen._common import collect_schemas
32+
from vgi.codegen._common import EXTRA_RESPONSE_TYPES, REQUEST_TYPES, collect_schemas
3333
from vgi.codegen.rust_schemas import emit, snake_case
3434

3535

@@ -263,7 +263,10 @@ def test_emitted_schemas_round_trip_to_pyarrow() -> None:
263263
emit(buf)
264264
parsed = _parse_generated(buf.getvalue())
265265

266-
expected = {snake_case(es.name): es for es in collect_schemas()}
266+
expected = {
267+
snake_case(es.name): es
268+
for es in collect_schemas(extra_response_types=(*EXTRA_RESPONSE_TYPES, *REQUEST_TYPES))
269+
}
267270
assert set(parsed) == set(expected), (
268271
f"emitted factories differ from collect_schemas(): "
269272
f"only-in-rust={sorted(set(parsed) - set(expected))}, "

‎tests/test_generated_ts_schemas.py‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import pyarrow as pa
2222
import pytest
2323

24-
from vgi.codegen._common import collect_schemas
24+
from vgi.codegen._common import EXTRA_RESPONSE_TYPES, REQUEST_TYPES, collect_schemas
2525
from vgi.codegen.ts_schemas import emit
2626

2727

@@ -99,6 +99,8 @@ def _parse_type(expr: str) -> pa.DataType:
9999
"float64()": pa.float64(),
100100
"utf8()": pa.string(),
101101
"binary()": pa.binary(),
102+
"largeUtf8()": pa.large_string(),
103+
"largeBinary()": pa.large_binary(),
102104
}
103105
if expr in simple:
104106
return simple[expr]
@@ -187,7 +189,10 @@ def test_checked_in_generated_ts_matches_generator() -> None:
187189
pytest.skip(f"{path} not found; set VGI_TS_GENERATED_TS or check out vgi-typescript next to vgi-python")
188190

189191
actual = _parse_generated_ts(path.read_text())
190-
expected = {es.name: es.schema for es in collect_schemas()}
192+
expected = {
193+
es.name: es.schema
194+
for es in collect_schemas(extra_response_types=(*EXTRA_RESPONSE_TYPES, *REQUEST_TYPES))
195+
}
191196

192197
missing = set(expected) - set(actual)
193198
extra = set(actual) - set(expected)
@@ -209,7 +214,10 @@ def test_parser_roundtrip_self_test() -> None:
209214
buf = io.StringIO()
210215
emit(buf)
211216
parsed = _parse_generated_ts(buf.getvalue())
212-
expected = {es.name: es.schema for es in collect_schemas()}
217+
expected = {
218+
es.name: es.schema
219+
for es in collect_schemas(extra_response_types=(*EXTRA_RESPONSE_TYPES, *REQUEST_TYPES))
220+
}
213221
assert set(parsed) == set(expected), "parser missed a factory the generator emitted"
214222
for name, schema in expected.items():
215223
assert schema.equals(parsed[name], check_metadata=False), (

0 commit comments

Comments
 (0)