MAINT: Formalize JSON-condition helper contract on MemoryInterface#1814
Merged
romanlutz merged 3 commits intoMay 26, 2026
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jsong468
reviewed
May 26, 2026
jsong468
approved these changes
May 26, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
What
Tightens the polymorphic contract for
_get_condition_json_property_matchand_get_condition_json_array_matchonMemoryInterface. The abstract decls already existed; this PR adds an explicit "Concrete subclasses translate this contract into their SQL dialect…" sentence to both abstract docstrings so the dialect-divergence reason for the polymorphism is captured in one place.Why
@abstractmethodand not "extract to a shared function"The two implementations have identical signatures but completely different SQL — SQLite uses
json_extract/json_each, Azure SQL usesJSON_VALUE/ISJSON/OPENJSON/JSON_QUERY. There is no shared logic to extract; every line is dialect-specific. The right way to formalize the polymorphism is an abstract method on the base, not a shared function or mixin. Callers inmemory_interface.py(e.g.get_attack_results,_get_condition_json_match) already dispatch throughself._get_condition_json_*— this PR just makes the contract those callers depend on explicit and well-documented in one place.Subclass docstrings are intentionally kept full (not collapsed to "see base class") because the doc builder and IDE hover don't auto-inherit docstrings — whatever is in the file is what renders.
Verified
uv run pytest tests/unit -x -q— 8049 passed, 118 skippeduv run pre-commit run --all-files— clean (ruff, ty, etc.)