feat(query): add canonical AST schema + OpenAPI generation for query DSL - #3330
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis change adds a versioned, strictly validated query explanation AST, integrates its schema version into explanation payloads, publishes it through OpenAPI, and adds corresponding generated TypeScript types and topology metadata. ChangesQuery AST contract
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant QueryExpressionExplanation
participant QueryExpressionExplanationAst
participant render_openapi.py
participant GeneratedTypeScript
QueryExpressionExplanation->>QueryExpressionExplanationAst: emit versioned explanation payload
QueryExpressionExplanationAst->>render_openapi.py: provide JSON schema
render_openapi.py->>GeneratedTypeScript: publish query AST contract
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem: polylogue-z9gh.3's design calls for "typed structured-plan lowering to one AST" and OpenAPI/JSON schema generation for the query-discovery vocabulary. explain_expression() already computes a compiled predicate tree and per-branch ast/lowering_plan dicts, but they were hand-rolled dict[str, object] shapes -- correct, but opaque to external tooling (an MCP client or OpenAPI-generated SDK only ever saw an untyped dict). This was the one named, not-yet-attempted size-M residual left on z9gh.3 after #3018, What changed: - New polylogue/archive/query/query_ast_schema.py: Pydantic models mirroring every predicate/pipeline-stage/clause dataclass's existing to_payload() shape one-to-one (QueryPredicateAst discriminated union, clause/ref-operand/ pipeline projections, QueryExpressionExplanationAst). This is a validating projection, not a second AST -- predicate_to_ast()/explanation_payload_to_ast() validate the dataclasses' own to_payload() output against the schema, so a producer that drifts from this shape fails loudly instead of silently diverging. Versioned as polylogue.query-explain-ast.v1, kept distinct from the existing polylogue.query-definition.v1 content-hashing protocol version (documented in the module docstring). - QueryExpressionExplanation.to_payload() now stamps schema_version -- the only new key; MCP explain(kind="query") and Polylogue.explain_query_expression() callers see it automatically. - devtools/render_openapi.py: publish QueryExpressionExplanationAst (with its full nested $defs) in docs/openapi/search.yaml, plus an x-polylogue-query-ast vendor extension documenting the schema version and how to obtain it live (MCP explain / Polylogue.explain_query_expression()). No new HTTP route was added -- there is no existing daemon route for query explain, so publishing schemas without a bound path keeps this PR to the documented residual rather than growing a new live surface. - Regenerated docs/openapi/search.yaml, webui/src/api/generated.ts (the typed TS client derives from it), and the topology projection/status docs (new module). Verification: - devtools test tests/unit/archive/query/test_query_ast_schema.py -- 28 passed: predicate<->AST round trip over 11 representative predicate shapes (field/not/and/or/exists/sequence/fts/semantic/lineage), full explanation-payload validation over 13 representative DSL expressions (compact field query, Boolean AND/OR, near: semantic, lineage:id:, exists, seq(), pipeline stages sort/limit/offset, group-by-count aggregate, JSON spec, durable-reference pipeline, terminal unit sources), a JSON-Schema buildability check, and two schema-drift rejection tests. - Ad hoc sweep of all 106 positive rows in archive/query/discovery.py's QUERY_DISCOVERY_EXAMPLES corpus validated cleanly against the new schema (0 failures) before writing the formal test file. - devtools test tests/unit/cli/test_query_expression.py tests/unit/archive/query/test_predicate_payload_roundtrip.py tests/unit/devtools/test_render_openapi.py tests/unit/api/test_facade_contracts.py -- 751 passed, 1 skipped (no regressions in explain/predicate/OpenAPI surfaces). - mypy --strict and ruff check/format clean on all touched files. - devtools render all --check: sync OK on every generated surface (openapi, webui-client, topology-status, and the rest). - Not run: the full non-slow suite / devtools verify --seed-testmon (stalled 600s into an unrelated corpus after cleanly finishing this PR's own tests -- consistent with heavy concurrent test load from other agent worktrees on this shared host, not a regression from this change). Ref polylogue-z9gh.3 Co-Authored-By: Claude <noreply@anthropic.com>
28c98a0 to
f324e06
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@polylogue/archive/query/query_ast_schema.py`:
- Around line 190-204: Update the public signatures of predicate_to_ast and
ast_to_predicate to use QueryPredicateAst: return QueryPredicateAst from
predicate_to_ast and accept ast: QueryPredicateAst in ast_to_predicate. Define
or use TypeAdapter[QueryPredicateAst] for validation, keeping the adapter
internally opaque only if required by the static checker while preserving the
typed public contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b9a04eb9-aabb-44ce-8ceb-4aacfaab20db
📒 Files selected for processing (8)
devtools/render_openapi.pydocs/openapi/search.yamldocs/plans/topology-target.yamldocs/topology-status.mdpolylogue/archive/query/expression.pypolylogue/archive/query/query_ast_schema.pytests/unit/archive/query/test_query_ast_schema.pywebui/src/api/generated.ts
| def predicate_to_ast(predicate: QueryPredicate) -> Any: | ||
| """Project a compiled predicate node into the canonical, typed AST. | ||
|
|
||
| This validates ``predicate``'s own lossless ``to_payload()`` projection | ||
| against :data:`QueryPredicateAst` -- it does not re-derive the payload by | ||
| walking the dataclass a second time, so the two shapes cannot drift apart | ||
| without a validation failure surfacing immediately. | ||
| """ | ||
| return _predicate_adapter.validate_python(predicate.to_payload()) | ||
|
|
||
|
|
||
| def ast_to_predicate(ast: Any) -> QueryPredicate: | ||
| """Invert :func:`predicate_to_ast` back into a typed predicate node.""" | ||
| payload = cast("dict[str, object]", ast.model_dump(mode="json", exclude_none=True)) | ||
| return predicate_from_payload(payload) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Pydantic v2 TypeAdapter generic parameter with Annotated discriminated union type annotation mypy
💡 Result:
Using Pydantic v2's TypeAdapter with Annotated discriminated union types often presents challenges for static type checkers like mypy [1]. The core issue is that while these constructs work correctly at runtime, mypy and other static checkers may struggle to resolve complex Annotated types or special forms (like Union and Annotated) when passed as values to TypeAdapter, often leading to type inference resulting in Any or triggering argument-type errors [2][1][3][4]. Current Status and Workarounds: 1. Explicit Annotations: When mypy raises an error during TypeAdapter instantiation, the standard workaround is to provide an explicit type annotation for the TypeAdapter instance itself [5]. python from typing import Union, Annotated from pydantic import TypeAdapter, Field # Define the discriminated union MyUnion = Annotated[Union[ModelA, ModelB], Field(discriminator='type')] # Instantiate with explicit annotation to satisfy mypy ta: TypeAdapter[Union[ModelA, ModelB]] = TypeAdapter(MyUnion) 2. Suppression: If type inference still fails or produces incorrect results (such as falling back to Any), it is common practice to use # type: ignore [5][1]. 3. Evolution of Support: Pydantic's internal typing mechanisms have evolved to address these issues. Some versions have intentionally relaxed internal typing to Any to prevent breaking changes in user code when Python's own typing rules shift [2][3]. Recent updates in the Pydantic ecosystem have also begun incorporating support for PEP 747 (TypeForm), which aims to provide native support for these patterns in static type checkers [1]. 4. Best Practices: - Avoid creating TypeAdapter instances dynamically inside functions [2]. Instead, define them as global constants and reference them where needed [2]. - If you encounter errors, ensure the Pydantic mypy plugin is correctly configured in your project, although note that the plugin primarily focuses on model-level validation and may not resolve all TypeAdapter inference limitations [6]. - If using discriminated unions inside lists, ensure the discriminator is applied to the union type itself (e.g., List[Annotated[Union[T1, T2], Field(discriminator='...')]]) rather than the list [7]. Because this limitation stems from how static checkers interact with Python's typing system and special forms, it is a known area of friction rather than a bug in your code [1][4]. Using explicit typing or suppression remains the recommended approach until static checkers provide broader, native support for these complex type forms [1].
Citations:
- 1: TypeAdapter and mypy compatibility pydantic/pydantic#7094
- 2:
TypeAdaptershould acceptAnnotated[T, ...]pydantic/pydantic#8735 - 3:
TypeAdapterwith Union hasAnytype hint argument starting with Pydantic 2.7.0+ pydantic/pydantic#9532 - 4: TypeAdapter[T] inferrence fails with mypy (quick fix) pydantic/pydantic#6615
- 5: https://pydantic.dev/docs/validation/latest/api/pydantic/type_adapter/
- 6: https://pydantic.dev/docs/validation/latest/integrations/dev-tools/mypy/
- 7: List of discriminated unions not working in pydantic 2.9 pydantic/pydantic#10352
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
sed -n '1,260p' polylogue/archive/query/query_ast_schema.py
echo
echo "== tooling configs =="
for f in pyproject.toml setup.cfg tox.ini mypy.ini pyrightconfig.json; do
[ -f "$f" ] && { echo "--- $f"; sed -n '1,220p' "$f"; }
done
echo
echo "== ast_to_predicate public usages =="
rg -n "ast_to_predicate|predicate_to_ast|_predicate_adapter|QueryPredicateAst" polylogue tests . --glob='*.py' || trueRepository: Sinity/polylogue
Length of output: 1964
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate typing behavior =="
python3 - <<'PY'
try:
from pydantic import TypeAdapter, BaseModel, Field
from typing import Annotated, Union
class A(BaseModel):
kind: str = "a"
x: int
class B(BaseModel):
kind: str = "b"
s: str
P = Annotated[Union[A, B], Field(discriminator="kind")]
t1: TypeAdapter[P] = TypeAdapter(P)
print("TypeAdapter[Annotated[Union[...], Field(discriminator=...)]] instantiation succeeded")
except Exception as e:
print(type(e).__name__, ":", e)
# Runtime serialization of None-bearing nested models omitted from output by ExcludeNone default-ish behavior.
class C(BaseModel):
kind: str = "c"
opt: int | None = None
model = C.model_validate(t1.validate_python({"kind": "c", "opt": None}))
print(model.model_dump(mode="json", exclude_none=True))
PYRepository: Sinity/polylogue
Length of output: 1964
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
sed -n '1,260p' polylogue/archive/query/query_ast_schema.py
echo
echo "== tooling configs =="
for f in pyproject.toml setup.cfg tox.ini mypy.ini pyrightconfig.json; do
[ -f "$f" ] && { echo "--- $f"; sed -n '1,220p' "$f"; }
done
echo
echo "== ast_to_predicate public usages =="
rg -n "ast_to_predicate|predicate_to_ast|_predicate_adapter|QueryPredicateAst" polylogue tests . --glob='*. py' || trueRepository: Sinity/polylogue
Length of output: 1964
Return/accept QueryPredicateAst instead of Any.
The module exposes a typed AST surface, but the two public functions currently erase that contract. Use TypeAdapter[QueryPredicateAst], -> QueryPredicateAst, and ast: QueryPredicateAst; if the project’s static checker rejects that exact generic form, keep the adapter internally opaque while still typing those public signatures to QueryPredicateAst.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@polylogue/archive/query/query_ast_schema.py` around lines 190 - 204, Update
the public signatures of predicate_to_ast and ast_to_predicate to use
QueryPredicateAst: return QueryPredicateAst from predicate_to_ast and accept
ast: QueryPredicateAst in ast_to_predicate. Define or use
TypeAdapter[QueryPredicateAst] for validation, keeping the adapter internally
opaque only if required by the static checker while preserving the typed public
contract.
1 participant
Summary
Adds a canonical, versioned Pydantic AST schema for the query DSL's compiled
predicate tree, unit-source pipelines, and lowering plan, and wires it into
devtools render openapiso the shape is published indocs/openapi/search.yaml(and the derivedwebui/src/api/generated.tsTypeScript client).
Problem
polylogue-z9gh.3's design explicitly calls for "typed structured-planlowering to one AST" and OpenAPI/JSON schema generation for the query
discovery vocabulary. Most of that program already shipped (#3018, #3066,
#3296), but the bead's own notes named exactly one residual not yet
attempted: "structured-plan -> canonical AST lowering + OpenAPI
generation" (sized M).
explain_expression()already computes a compiledpredicate tree and per-branch
ast/lowering_plandicts (seepolylogue/archive/query/expression.py:_ast_payload/_lowering_plan_payload/QueryExpressionExplanation.to_payload), but they were hand-rolleddict[str, object]shapes: correct, but opaque to external tooling. An MCPclient or an OpenAPI-generated SDK only ever saw an untyped dict with no
documented schema.
Solution
polylogue/archive/query/query_ast_schema.py: Pydantic modelsmirroring every existing predicate/pipeline-stage/clause dataclass's
to_payload()shape one-to-one —QueryPredicateAst(a discriminatedunion covering field/not/and-or/exists/sequence/fts/semantic/lineage
predicates), clause/ref-operand/pipeline-stage projections, and the
top-level
QueryExpressionExplanationAst. This is deliberately avalidating projection, not a second AST or a parallel IR:
predicate_to_ast()/explanation_payload_to_ast()validate thedataclasses' own existing
to_payload()output against the schema, so ifa producer's payload shape ever drifts from what's declared here, a
Pydantic
ValidationErrorsurfaces immediately in the new test filerather than the drift silently reaching an agent or generated client.
polylogue.query-explain-ast.v1(
QUERY_AST_SCHEMA_VERSION), kept a distinct axis from the existingpolylogue.query-definition.v1content-hashing protocol version inpolylogue/core/query_identity.py— the module docstring explains whythey must not be conflated (one versions the content-addressed predicate
grammar used for query hashing/identity, the other versions this new
broader discovery/explain envelope).
QueryExpressionExplanation.to_payload()now stampsschema_version—the only new key added to the existing payload shape. MCP
explain(kind="query")andPolylogue.explain_query_expression()callerspick it up automatically with no call-site changes.
devtools/render_openapi.py: publishesQueryExpressionExplanationAst(with its full nested
$defsgraph — every predicate/pipeline-stagevariant) as a component schema in
docs/openapi/search.yaml, plus a newx-polylogue-query-astvendor extension documenting the schema version,the root schema ref, and how to obtain a live instance (there's no
existing daemon HTTP route for query explain, only the MCP
explainoperation and the Python facade, so the extension says that explicitly
rather than implying a route that doesn't exist).
docs/openapi/search.yaml,webui/src/api/generated.ts(thetyped TS client derives from the OpenAPI doc), and
docs/plans/topology-target.yaml/docs/topology-status.md(new moduleunder
polylogue/, per this repo's topology-projection convention).What's covered vs. deferred
Covered: a documented, versioned, round-trip-tested AST for the
SessionQuerySpec.boolean_predicatetree (the actual "structured plan" acompiled query lowers to), the clause/pipeline/unit-source/reference-operand
projections
explain_expression()already emits, and OpenAPI publication ofall of it.
Deferred (explicitly out of scope for this residual, not silently dropped):
no new HTTP route for query-explain was added (MCP + Python facade remain
the only live surfaces); the
unit_source.pipeline_stagestransform-stagevocabulary member (
QueryUnitTransformStageAst) is modeled but has noproducer yet (matches upstream's own "reserved vocabulary member" note on
QueryUnitTransformStage); this PR does not touch execution-layer gaps(those were already handled separately in #3296).
Verification
devtools test tests/unit/archive/query/test_query_ast_schema.py— 28passed: predicate↔AST round trip over 11 representative predicate shapes
(field/not/and/or/exists/sequence/fts/semantic/lineage), full
explanation-payload validation over 13 representative DSL expressions
(compact field query, Boolean AND/OR,
near:"..."semantic,lineage:id:,exists,seq(...), pipeline stages sort/limit/offset, group-by-countaggregate, raw JSON spec, durable-reference pipeline
from result-set:... | ..., terminal unit sources), a JSON-Schemabuildability check, and two schema-drift rejection tests.
writing it) of all 106 positive rows in
archive/query/discovery.py:QUERY_DISCOVERY_EXAMPLESvalidated cleanlyagainst the new schema with 0 failures.
devtools test tests/unit/cli/test_query_expression.py tests/unit/archive/query/test_predicate_payload_roundtrip.py tests/unit/devtools/test_render_openapi.py tests/unit/api/test_facade_contracts.py— 751 passed, 1 skipped (noregressions in the explain/predicate/OpenAPI surfaces touched by this
change).
mypy --strictandruff check/formatclean on all touched files.devtools render all --check— sync OK on every generated surface(openapi, webui-client, topology-status, and the rest).
devtools verify --seed-testmon(it stalled 600s into an unrelated part of the corpus, after cleanly
finishing every test in this PR's own file — consistent with heavy
concurrent test load from other agent worktrees sharing this host at the
time, not a regression introduced here). The pre-push hook's
devtools verify --quick(format + lint + mypy + render-all-check) ranand passed on push.
Ref polylogue-z9gh.3
Summary by CodeRabbit
New Features
Documentation
Tests