Skip to content

feat: add filter inspection, manipulation, and builder API - #35

Merged
rcleveng merged 16 commits into
mainfrom
claude/add-filter-inspection-api-KbZnx
Mar 25, 2026
Merged

rcleveng merged 16 commits into
mainfrom
claude/add-filter-inspection-api-KbZnx

Conversation

@rcleveng

Copy link
Copy Markdown
Owner

Summary

  • Add structured AST (FilterExpression, Comparison, AndExpression, OrExpression, NotExpression) for parsed AIP-160 filters with inspection (get_fields), mutation (rename_field, remove, extract), serialization (__str__), and combining operators (&, |, ~)
  • Add FilterBuilder for fluent programmatic filter construction
  • parse_filter() now returns FilterExpression; apply_filter() accepts FilterExpression | str | None

Test plan

  • All 65 existing tests pass unchanged
  • 91 new AST tests: parsing, serialization, get_fields, rename_field, remove, extract, combining, FilterBuilder
  • 9 new DB integration tests: parse → manipulate → apply_filter with real SQLite queries
  • Verify CI passes

https://claude.ai/code/session_017DCbAVeWffb6QjXvt4en78

claude added 2 commits March 25, 2026 04:23
Expose the internally-parsed AIP-160 AST via a structured public API so
consumers can inspect, manipulate, and construct filter expressions without
fragile regex/string manipulation.

New modules:
- ast.py: FilterExpression, Comparison, AndExpression, OrExpression,
  NotExpression, Operator, and Value types with serialization, get_fields,
  rename_field, extract, remove, and combining operators (&, |, ~)
- builder.py: FilterBuilder for programmatic filter construction

Changes to existing code:
- parse_filter() now returns FilterExpression (internal Lark parsing renamed
  to _parse_lark_tree)
- apply_filter() accepts FilterExpression in addition to str
- Expanded __init__.py exports

https://claude.ai/code/session_017DCbAVeWffb6QjXvt4en78
Copilot AI review requested due to automatic review settings March 25, 2026 18:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a first-class AST and builder API for AIP-160 filters so callers can parse filters into a structured representation, inspect/mutate them, serialize them back to strings, and pass them directly into apply_filter().

Changes:

  • Added AST types (FilterExpression, Comparison, logical nodes, value nodes) with inspection/mutation/serialization and boolean/composition operators.
  • Added FilterBuilder for fluent programmatic construction of filters.
  • Updated parsing/application APIs (parse_filter() returns FilterExpression; apply_filter() accepts FilterExpression | str | None) and expanded test coverage including DB integration.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/sqlalchemy_aip160/ast.py New AST node model, serialization, inspection/mutation helpers, and combining operators.
src/sqlalchemy_aip160/builder.py New fluent FilterBuilder for constructing FilterExpression programmatically.
src/sqlalchemy_aip160/aip160_filter.py Adds AST parsing path, internal _parse_lark_tree(), and allows apply_filter() to accept FilterExpression.
src/sqlalchemy_aip160/__init__.py Exposes new public API surface (AST types, builder, errors, parse/apply functions).
tests/test_ast.py New unit tests for AST parsing/serialization/inspection/mutation/combining and builder behavior.
tests/test_integration.py New SQLite integration tests for parse → manipulate → apply_filter() end-to-end.
tests/test_aip160_filter.py Adjusts legacy tests to use _parse_lark_tree() now that parse_filter() returns FilterExpression.
.gitignore Adds basic Python build/artifact ignores.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/sqlalchemy_aip160/ast.py Outdated
Comment thread src/sqlalchemy_aip160/ast.py Outdated
Comment thread tests/test_integration.py Outdated
Comment thread tests/test_integration.py Outdated
Comment thread src/sqlalchemy_aip160/aip160_filter.py
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 25, 2026 19:04

Copilot AI commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

@rcleveng I've opened a new pull request, #36, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

@rcleveng I've opened a new pull request, #37, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/sqlalchemy_aip160/aip160_filter.py
Comment thread tests/test_integration.py
Comment thread src/sqlalchemy_aip160/builder.py Outdated
Copilot AI and others added 2 commits March 25, 2026 12:18
…ply_filter docstring (#37)

* Initial plan

* docs: restore relationship-filtering and field_aliases examples in apply_filter docstring

Co-authored-by: rcleveng <1906807+rcleveng@users.noreply.github.com>
Agent-Logs-Url: https://github.com/rcleveng/sqlalchemy_aip160/sessions/e6191d18-0d2d-47cd-9649-73d5aa5fa5a9

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: rcleveng <1906807+rcleveng@users.noreply.github.com>
* Initial plan

* fix: use deepcopy in __and__/__or__/__invert__ to prevent aliasing bugs

Co-authored-by: rcleveng <1906807+rcleveng@users.noreply.github.com>
Agent-Logs-Url: https://github.com/rcleveng/sqlalchemy_aip160/sessions/8da6e4cf-23eb-4500-a944-e989372bb400

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: rcleveng <1906807+rcleveng@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 25, 2026 19:20
rcleveng and others added 2 commits March 25, 2026 12:20
Add TestExtractPseudoFieldPattern that mirrors the actual usage pattern:
extract a pseudo-field (starred=true/false), read its boolean value, and
apply the remaining filter to a DB query. Covers starred at start/middle/end,
starred-only, no starred, None/empty input, unquoted values, and chaining
multiple pseudo-field extractions (label + starred).

https://claude.ai/code/session_017DCbAVeWffb6QjXvt4en78

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +616 to +622
def arg(self, items: list) -> Any:
item = items[0]
# When an arg is a comparable/member (e.g. bare identifiers like `true`),
# it arrives as a plain string. Wrap it as a StringValue.
if isinstance(item, str):
return StringValue(value=item)
return item

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ASTTransformer.arg() wraps any bare identifier argument (e.g. true) into StringValue, which then serializes as a quoted string ("true"). This loses the original literal form and makes parse_filter('is_active = true') serialize to is_active = "true" even though AIP-160 commonly treats booleans as unquoted literals. Consider introducing a dedicated boolean value node (and parsing true/false into it) so serialization can emit true/false without quotes, and update coercion accordingly.

Copilot uses AI. Check for mistakes.
Comment thread src/sqlalchemy_aip160/ast.py Outdated
Comment thread tests/test_aip160_filter.py Outdated
Comment thread src/sqlalchemy_aip160/aip160_filter.py Outdated
  Make FilterBuilder.build() return deep-copied AST nodes so mutating a
  built FilterExpression does not affect later builds from the same
  builder. Add a regression test for builder independence and expand the
  README with installation, usage, aliases, inspection, manipulation, and
  programmatic filter-building examples.
Copilot AI review requested due to automatic review settings March 25, 2026 19:52
rcleveng and others added 2 commits March 25, 2026 12:53
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/sqlalchemy_aip160/aip160_filter.py Outdated
rcleveng and others added 2 commits March 25, 2026 12:57
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 25, 2026 19:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_ast.py Outdated

def test_invert_empty(self):
a = parse_filter("")
assert (~a).root is None

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FilterExpression.__invert__ raises ValueError when root is None (empty/match-all), but this test asserts ~parse_filter("") returns another empty expression. Either update the test to expect a ValueError (recommended, since match-none can’t be represented with root=None), or change __invert__ semantics to handle empty explicitly.

Suggested change
assert (~a).root is None
with pytest.raises(ValueError):
~a

Copilot uses AI. Check for mistakes.
The API docs state the following:

  API documentation and examples should encourage the use of explicit
  parentheses to avoid confusion, but should not require explicit
  parentheses.

So we'll add it in non-required cases when we generate the filter code.
Copilot AI review requested due to automatic review settings March 25, 2026 21:46
@rcleveng
rcleveng merged commit 0244ba2 into main Mar 25, 2026
@rcleveng
rcleveng deleted the claude/add-filter-inspection-api-KbZnx branch March 25, 2026 21:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants