DEV1260: Variables in Filters

[AivanF]

Summary by CodeRabbit

Release Notes

• New Features

  • Filters now support {variable} placeholders for dynamic values sourced from a new variables query parameter.
  • String and number values are inserted directly; escape literal braces with {{ and }}.
  • Undefined variables raise errors during processing.

• Documentation

  • Updated query documentation with filter variable details and placeholder syntax.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request adds support for variable placeholder substitution in filter expressions. Filter strings can now reference {variable_name} placeholders sourced from query.variables: Dict[str, Any], with support for escaping literal braces via {{/}}. Missing or invalid variables raise errors during substitution.

Changes

Cohort / File(s)	Summary
Documentation CLAUDE.md, docs/concepts/queries.md	Updated documentation to describe the new {variable_name} placeholder syntax in filters, the new optional variables query parameter, and placeholder substitution behavior including escaping rules and error handling.
Core Implementation slayer/core/query.py	Added substitute_variables() utility function with regex-based placeholder detection and validation; added optional variables: Optional[Dict[str, Any]] field to SlayerQuery model for providing substitution values. Validates that variable names match allowed identifier pattern, values are string/int/float, and referenced variables are defined.
Engine Integration slayer/engine/enrichment.py	Integrated variable substitution into query enrichment pipeline; filters are substituted when query.variables is present, with substituted filters replacing original values in downstream processing (filter transformation extraction, column resolution, WHERE/HAVING classification).
Tests tests/test_models.py	Added comprehensive TestSubstituteVariables test suite validating placeholder substitution for various data types, multiple placeholders per filter, escaped brace handling, and error cases for undefined variables, invalid names, and invalid value types. Includes test confirming SlayerQuery.variables field is accepted and retained.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Filter strings now bloom with variables true,
{placeholders} dance in queries new,
Escaped braces double, like our twitching nose,
From source to enrichment, the substitution flows!
No more hardcoding—flexibility grows! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'DEV1260: Variables in Filters' directly and accurately summarizes the main change: adding support for variable placeholders in filter expressions across the codebase.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch aivan/dev-1260-variables-in-filters

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

slayer/engine/enrichment.py (1)

441-450: Consider moving the import to the top of the file.

The substitute_variables import is placed inside the conditional block, but slayer.core.query is already imported at line 27 (from slayer.core.query import SlayerQuery). Moving this import to the top with the other imports from the same module would be more consistent with the coding guidelines and wouldn't risk circular import issues.

As per coding guidelines: "Place imports at the top of files"

Suggested change

At line 27, extend the existing import:

-from slayer.core.query import SlayerQuery
+from slayer.core.query import SlayerQuery, substitute_variables

Then simplify lines 443-448:

     if query.variables and query_filters:
-        from slayer.core.query import substitute_variables
-
         query_filters = [
             substitute_variables(filter_str=f, variables=query.variables) for f in query_filters
         ]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@slayer/engine/enrichment.py` around lines 441 - 450, Move the from
slayer.core.query import substitute_variables import to the top of
slayer/engine/enrichment.py alongside the existing from slayer.core.query import
SlayerQuery import so all imports from slayer.core.query are co-located; then
remove the in-block import and keep the conditional logic that calls
substitute_variables (used with query.filters and query.variables) unchanged,
ensuring you only reference substitute_variables in the query-level filter
substitution block.

tests/test_models.py (1)

571-579: Consider adding an integration test for the full enrichment flow.

The unit tests thoroughly cover substitute_variables in isolation, but test_variable_in_slayer_query only verifies that the variables field is accepted and stored on SlayerQuery. There's no test verifying that variables are actually substituted during query enrichment and affect the final SQL output.

An integration test in tests/integration/ that runs a query with filters containing placeholders and variables providing values would catch regressions in the enrichment.py integration point (lines 443-448).

Would you like me to draft an integration test case that verifies the end-to-end flow?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_models.py` around lines 571 - 579, Add an integration test that
verifies variables are substituted through the full enrichment flow: create a
SlayerQuery with a filters string containing a placeholder like "{val}" and
variables={"val":"completed"}, pass that SlayerQuery into the enrichment
entrypoint that uses substitute_variables (the function in enrichment.py that
consumes SlayerQuery and returns the enriched SQL), then assert the
returned/enriched SQL contains the substituted value (e.g., "status =
'completed'") to ensure end-to-end substitution works.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@slayer/engine/enrichment.py`:
- Around line 441-450: Move the from slayer.core.query import
substitute_variables import to the top of slayer/engine/enrichment.py alongside
the existing from slayer.core.query import SlayerQuery import so all imports
from slayer.core.query are co-located; then remove the in-block import and keep
the conditional logic that calls substitute_variables (used with query.filters
and query.variables) unchanged, ensuring you only reference substitute_variables
in the query-level filter substitution block.

In `@tests/test_models.py`:
- Around line 571-579: Add an integration test that verifies variables are
substituted through the full enrichment flow: create a SlayerQuery with a
filters string containing a placeholder like "{val}" and
variables={"val":"completed"}, pass that SlayerQuery into the enrichment
entrypoint that uses substitute_variables (the function in enrichment.py that
consumes SlayerQuery and returns the enriched SQL), then assert the
returned/enriched SQL contains the substituted value (e.g., "status =
'completed'") to ensure end-to-end substitution works.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6469f151-9e61-4595-b5a7-4d69c4edf782

📥 Commits

Reviewing files that changed from the base of the PR and between bdce4d8 and c06d069.

📒 Files selected for processing (5)

• CLAUDE.md
• docs/concepts/queries.md
• slayer/core/query.py
• slayer/engine/enrichment.py
• tests/test_models.py