Skip to content

Conversation

@AbdelrhmanBassiouny
Copy link
Collaborator

This pull request refactors the entity query language implementation to improve modularity, clarity, and maintainability. The main changes include moving quantifier logic to a dedicated module, simplifying and reorganizing matching logic, introducing new selection and matching patterns in the documentation, and making several type and utility improvements. These changes collectively make the codebase easier to extend and understand, and provide clearer guidance in the documentation for users.

API and Codebase Refactoring

  • Moved quantifier functions (an, a, the) and related logic from entity.py into a new module quantify_entity.py, improving separation of concerns and modularity. Test imports were updated accordingly. [1] [2] [3] [4] [5] [6]
  • Removed Match, MatchEntity, match, and entity_matching implementations from entity.py (now imported from a new module), streamlining the core entity logic.

Documentation and Usability

  • Expanded the documentation in examples/eql/match.md with new sections and examples for select(), match_any(), and select_any() to illustrate how to select and match inner objects and elements in collections. This helps users understand advanced query patterns.

Type and Utility Improvements

  • Added is_iterable_type() utility function to check if a type is iterable, and improved make_list() usage for more robust type handling. [1] [2] [3]
  • Refactored variable tracking classes by introducing a new Selectable base class and updating CanBehaveLikeAVariable to inherit from it, clarifying responsibilities and improving extensibility.

Minor Enhancements and Cleanups

  • Added an operand_value property to OperationResult for easier access to operand values.
  • Minor formatting and message improvements in failure classes.
  • Updated test files to use the new quantifier imports and removed legacy match query tests that are now covered elsewhere.

These changes collectively modernize the query language implementation and make it easier for new contributors and users to understand and use advanced query features.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request refactors the entity query language to improve modularity by extracting quantifier and matching logic into dedicated modules. The refactoring moves quantifier functions (an, a, the) to a new quantify_entity.py module and matching functions (match, entity_matching, select, match_any, select_any) to a new match.py module, making the codebase more organized and easier to extend.

  • Introduced new modules quantify_entity.py and match.py to house previously scattered functionality
  • Enhanced the matching API with new select(), match_any(), and select_any() functions for more expressive queries
  • Updated type annotations and utilities including a new is_iterable_type() function and refactored Selectable base class

Reviewed changes

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

Show a summary per file
File Description
src/krrood/entity_query_language/quantify_entity.py New module containing quantifier functions (an, a, the) previously in entity.py
src/krrood/entity_query_language/match.py New module containing all matching logic (Match, MatchEntity, Select classes and related functions)
src/krrood/entity_query_language/entity.py Removed quantifier and match functions, streamlined to core entity logic
src/krrood/entity_query_language/symbolic.py Added operand_value property, introduced Selectable base class, improved Literal initialization
src/krrood/entity_query_language/utils.py Added is_iterable_type() utility function
test/test_eql/test_match.py New test file for match functionality
test/test_eql/test_core/test_queries.py Removed legacy match test (moved to dedicated test file)
test/test_* (multiple files) Updated imports to use new quantify_entity module
examples/eql/match.md Added documentation for select(), match_any(), and select_any() functions
examples/eql/* (multiple files) Updated imports to use new quantify_entity module

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

Comment on lines +131 to +134
## Selecting inner objects with `select()`

Use `select(Type)` when you want the matched inner objects to appear in the result. The evaluation then
returns a mapping from the selected variables to the concrete objects (a unification dictionary).
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The documentation sections for select(), match_any(), and select_any() are duplicated. Lines 131-207 are identical to lines 209-285. Remove the duplicate sections (lines 209-285).

Copilot uses AI. Check for mistakes.

def _get_or_create_variable(self) -> CanBehaveLikeAVariable[T]:
"""
Create a variable with the given type if
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The docstring is incomplete. It should describe what the method does fully, e.g., 'Create a variable with the given type if one does not already exist, otherwise return the existing variable.'

Suggested change
Create a variable with the given type if
Return the existing variable if it exists; otherwise, create a new variable with the given type and return it.
:return: The existing variable if present, otherwise a newly created variable of the given type.

Copilot uses AI. Check for mistakes.
from typing_extensions import Dict, List

from krrood.entity_query_language.entity import an, entity, let, From
from krrood.entity_query_language.entity import entity, let, From
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

Import of 'From' is not used.

Suggested change
from krrood.entity_query_language.entity import entity, let, From
from krrood.entity_query_language.entity import entity, let

Copilot uses AI. Check for mistakes.
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.

1 participant