feat(conditionals): extensible operator engine + in/string/existence operators + grouping (#128)#129
Merged
Merged
Conversation
…ne (#128) Rewire the internal ConditionalEvaluator facade to delegate Evaluate(string, IEvaluationContext) to the new condition-expression engine (lexer, Pratt parser, ConditionEvaluatorCore) using the Default dialect, instead of its own flat-token evaluator. Public signatures and behavior are unchanged. Also fixes two engine gaps surfaced by the full compatibility-gate test run: - ConditionEvaluatorCore now falls back to a variable's literal path text when it is used as a comparison operand and does not resolve (e.g. the bareword `Active` in `Status = Active`), matching the legacy evaluator's ResolveValueOrLiteral behavior, while leaving standalone/logical-operator truthiness checks strict (missing variable still evaluates to false). - ConditionLexer now accepts `@` as an identifier character so loop-metadata tokens (@FIRST, @last, @index, @count) tokenize correctly instead of being rejected as an unknown operator. Validate's IsComparisonOperator now also recognizes the new infix word-operators (in, contains, startswith, endswith) so they aren't flagged as UnknownOperator.
…tor check (#128) Validate() classified exists/is/empty as "comparison", so an expression ending in one of these keywords tripped the trailing-operator MissingOperand check. Give them their own "postfix" classification instead, which the existing structural checks already treat as neither comparison nor logical.
…emove legacy Expressions (#128)
… edges (#128) - F1: Validate now parses via the engine (accepts in/contains/exists/... and the previously-broken 'in (...)' with spaces); keeps empty/unbalanced-quote pre-checks and falls back to the legacy heuristic for typed failure classes. - F2/F3/F4: lock current post-migration behavior with characterization tests and add a 'Reserved words & literal quoting' note to the author/developer docs. - F5: remove dead IsKnownOperatorToken / _allTokens from ConditionOperatorRegistry. All 28 ConditionValidationTests pass unedited; full suite 1175 green; format clean.
vaceslav
force-pushed
the
feat/128-extensible-condition-operators
branch
from
July 21, 2026 07:20
2df6c2d to
2de83c9
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors Templify’s conditional/boolean evaluation by replacing the legacy Expressions/ boolean-expression parser and the existing conditional evaluator internals with a unified lexer→parser→AST→operator-registry→evaluator engine under TriasDev.Templify/Conditionals/Engine/. It also extends the supported condition syntax with membership, string, existence/emptiness operators, and parenthesized grouping, and updates tests/docs accordingly.
Changes:
- Introduces a new extensible condition engine (lexer, Pratt parser, AST, operator registry, dialect-aware evaluator) and migrates both
{{#if ...}}and inline{{(...)}}evaluation to it. - Adds new operators:
in,contains,startswith,endswith,exists,is empty,is not empty, plus grouping and list literals. - Updates public API docs + project documentation and adds broad unit/integration test coverage for the new engine and operators.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| TriasDev.Templify/Visitors/PlaceholderVisitor.cs | Migrates inline {{(...)}} evaluation to the new condition engine. |
| TriasDev.Templify/Expressions/EvaluationContextAdapter.cs | Removes legacy inline-expression context adapter (engine removed). |
| TriasDev.Templify/Expressions/BooleanExpressionParser.cs | Removes legacy inline-expression parser. |
| TriasDev.Templify/Expressions/BooleanExpression.cs | Removes legacy expression AST/evaluator types. |
| TriasDev.Templify/Conditionals/IConditionEvaluator.cs | Updates XML docs to include new operators. |
| TriasDev.Templify/Conditionals/Engine/Operators/StringOperators.cs | Adds contains / startswith / endswith operators. |
| TriasDev.Templify/Conditionals/Engine/Operators/MembershipOperator.cs | Adds in membership operator. |
| TriasDev.Templify/Conditionals/Engine/Operators/LogicalOperators.cs | Adds logical operators with precedence for Pratt parsing. |
| TriasDev.Templify/Conditionals/Engine/Operators/ExistenceOperators.cs | Adds postfix exists / is empty / is not empty operators. |
| TriasDev.Templify/Conditionals/Engine/Operators/ComparisonOperators.cs | Implements =, !=, <, >, <=, >= operators using dialect comparison rules. |
| TriasDev.Templify/Conditionals/Engine/OperatorFixity.cs | Defines prefix/infix/postfix operator fixity. |
| TriasDev.Templify/Conditionals/Engine/IConditionOperator.cs | Defines operator contract for extensibility. |
| TriasDev.Templify/Conditionals/Engine/ConditionValueOps.cs | Adds shared equality/string coercion helpers used by operators. |
| TriasDev.Templify/Conditionals/Engine/ConditionToken.cs | Introduces token model for lexing. |
| TriasDev.Templify/Conditionals/Engine/ConditionParser.cs | Adds Pratt parser and parse exception type. |
| TriasDev.Templify/Conditionals/Engine/ConditionOperatorRegistry.cs | Central operator registration/lookup for parsing/evaluation. |
| TriasDev.Templify/Conditionals/Engine/ConditionNode.cs | Defines AST node types (literal/variable/list/operator). |
| TriasDev.Templify/Conditionals/Engine/ConditionLexer.cs | Adds lexer/tokenizer for condition expressions. |
| TriasDev.Templify/Conditionals/Engine/ConditionEvaluatorCore.cs | Adds AST evaluator core + dialect integration. |
| TriasDev.Templify/Conditionals/Engine/ConditionDialect.cs | Introduces Default vs Inline dialect semantic profiles. |
| TriasDev.Templify/Conditionals/ConditionalEvaluator.cs | Rewires {{#if}}/standalone evaluation and validation to new engine. |
| TriasDev.Templify.Tests/Integration/NewOperatorsIntegrationTests.cs | Adds end-to-end coverage for new operators via public API and document pipeline. |
| TriasDev.Templify.Tests/Expressions/BooleanExpressionParserTests.cs | Removes tests for the legacy inline expression parser (engine removed). |
| TriasDev.Templify.Tests/Engine/StringOperatorsTests.cs | Adds unit tests for string operators. |
| TriasDev.Templify.Tests/Engine/NewSyntaxValidationTests.cs | Adds tests ensuring Validate accepts new syntax. |
| TriasDev.Templify.Tests/Engine/MembershipOperatorTests.cs | Adds unit tests for in operator forms and negation. |
| TriasDev.Templify.Tests/Engine/InlineDialectTests.cs | Adds tests covering inline dialect behavior and parity. |
| TriasDev.Templify.Tests/Engine/ExistenceValidationTests.cs | Adds validation tests for postfix existence/emptiness syntax. |
| TriasDev.Templify.Tests/Engine/ExistenceOperatorsTests.cs | Adds unit tests for exists / is empty / is not empty semantics. |
| TriasDev.Templify.Tests/Engine/ConditionValueOpsTests.cs | Adds tests for shared equality semantics. |
| TriasDev.Templify.Tests/Engine/ConditionParserTests.cs | Adds tests for precedence/grouping + malformed parsing. |
| TriasDev.Templify.Tests/Engine/ConditionLexerTests.cs | Adds tests for lexing behavior. |
| TriasDev.Templify.Tests/Engine/ConditionEvaluatorCoreTests.cs | Adds tests for evaluator core and basic operators. |
| TriasDev.Templify.Tests/Engine/CompatEdgeCharacterizationTests.cs | Adds characterization tests locking accepted edge-case behavior changes. |
| docs/superpowers/specs/2026-07-20-extensible-condition-operators-design.md | Adds design spec for the unified engine and new operators. |
| docs/superpowers/plans/2026-07-20-extensible-condition-operators.md | Adds implementation plan for the refactor and operator additions. |
| docs/for-template-authors/template-syntax.md | Documents new operators + precedence for template authors. |
| docs/for-template-authors/conditionals.md | Documents membership/string/existence operators and grouping for {{#if}}. |
| docs/for-template-authors/boolean-expressions.md | Documents new operators, reserved words, and precedence for {{(...)}}. |
| docs/for-developers/condition-evaluation.md | Documents new operator reference and reserved-word/literal rules for developers. |
| docs/FAQ.md | Updates feature/operator list in FAQ. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
84
to
86
| catch (Conditionals.Engine.ConditionParseException) | ||
| { | ||
| _warningCollector.AddWarning(ProcessingWarning.ExpressionFailed(placeholder.VariableName, "Failed to parse expression")); |
Comment on lines
+16
to
+29
| public bool Evaluate(ConditionEvaluatorCore core, IReadOnlyList<ConditionNode> operands) | ||
| { | ||
| object? left = core.EvaluateValue(operands[0]); | ||
| object? right = core.EvaluateValue(operands[1]); | ||
|
|
||
| foreach (object? candidate in EnumerateCandidates(right)) | ||
| { | ||
| if (ConditionValueOps.AreEqual(left, candidate)) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } |
Comment on lines
+12
to
+17
| public bool Evaluate(ConditionEvaluatorCore core, IReadOnlyList<ConditionNode> operands) | ||
| { | ||
| string left = ConditionValueOps.ToStr(core.EvaluateValue(operands[0])); | ||
| string right = ConditionValueOps.ToStr(core.EvaluateValue(operands[1])); | ||
| return Test(left, right); | ||
| } |
Comment on lines
+6
to
+10
| /// <summary> | ||
| /// Dialect-independent value helpers shared by operators (equality, string coercion). | ||
| /// These preserve the historical <c>ConditionalEvaluator</c> equality semantics so that | ||
| /// <c>=</c>, <c>in</c>, and the string operators behave identically regardless of dialect. | ||
| /// </summary> |
Comment on lines
+6
to
+22
| internal sealed class OrOperator : IConditionOperator | ||
| { | ||
| public IReadOnlyList<string> Tokens { get; } = new[] { "or" }; | ||
| public int Precedence => 1; | ||
| public OperatorFixity Fixity => OperatorFixity.Infix; | ||
| public bool Evaluate(ConditionEvaluatorCore core, IReadOnlyList<ConditionNode> operands) | ||
| => core.EvaluateBool(operands[0]) || core.EvaluateBool(operands[1]); | ||
| } | ||
|
|
||
| internal sealed class AndOperator : IConditionOperator | ||
| { | ||
| public IReadOnlyList<string> Tokens { get; } = new[] { "and" }; | ||
| public int Precedence => 2; | ||
| public OperatorFixity Fixity => OperatorFixity.Infix; | ||
| public bool Evaluate(ConditionEvaluatorCore core, IReadOnlyList<ConditionNode> operands) | ||
| => core.EvaluateBool(operands[0]) && core.EvaluateBool(operands[1]); | ||
| } |
…ine error handling (#128) Addresses Copilot review findings on the condition engine (#129): - Correct ConditionValueOps XML summary: it is dialect-independent and used by `in`/string operators and DefaultConditionDialect.AreEqual; it does NOT govern `=`/`!=` in the Inline dialect (object.Equals). - Add ConditionEvaluatorCore.ResolveValueStrict: resolves a VariableNode via TryResolveVariable (null when missing) instead of the path-text literal fallback used by comparison operators. - InOperator and the string operators (contains/startswith/endswith) now use ResolveValueStrict so a missing variable never becomes its own path name for matching; a missing `in` collection yields no candidates, and a missing string operand short-circuits to false. - Dedup: IsEmptyOperator/IsNotEmptyOperator now use the shared ResolveValueStrict instead of a private ResolveOperandValue helper. ExistsOperator is unchanged (still needs presence, not value). - PlaceholderVisitor: inline expression failures now include the actual exception message and also catch ArgumentException/ InvalidOperationException/InvalidCastException as non-fatal warnings, restoring pre-migration resilience. Adds TriasDev.Templify.Tests/Engine/StrictResolutionTests.cs (6 tests). Full suite: 1181 passed (was 1175).
Contributor
Author
|
Thanks for the review 🤖 — went through all five. Summary of what changed in Addressed in code:
Added On the |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 42 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
TriasDev.Templify/Conditionals/Engine/Operators/LogicalOperators.cs:19
andis given higher precedence thanor(OrOperator.Precedence=1, AndOperator.Precedence=2), which makesA or B and Cparse asA or (B and C). Issue #128 explicitly requires keepingand/orat equal precedence (left-to-right) to avoid silently changing semantics of already-stored conditions/templates; this precedence change therefore violates the linked acceptance criteria.
internal sealed class AndOperator : IConditionOperator
{
public IReadOnlyList<string> Tokens { get; } = new[] { "and" };
public int Precedence => 2;
public OperatorFixity Fixity => OperatorFixity.Infix;
Comment on lines
+37
to
+51
| if (c == '"') | ||
| { | ||
| i++; | ||
| StringBuilder sb = new(); | ||
| while (i < text.Length && text[i] != '"') | ||
| { | ||
| if (text[i] == '\\' && i + 1 < text.Length && text[i + 1] == '"') | ||
| { sb.Append('"'); i += 2; continue; } | ||
| sb.Append(text[i]); | ||
| i++; | ||
| } | ||
| i++; // closing quote (if present) | ||
| tokens.Add(new ConditionToken(ConditionTokenType.String, sb.ToString())); | ||
| continue; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Good catch — fixed in 6f7c816. The legacy inline parser did accept both quote styles, so single-quoted string literals are now supported again on the inline {{(...)}} path: ConditionLexer gained an opt-in allowSingleQuotedStrings flag that PlaceholderVisitor enables (with \' escape support). The {{#if}}/text/standalone path keeps the flag off, so its lexing is byte-for-byte unchanged (it never supported single quotes). Added SingleQuoteLexingTests; full suite 1187/1187, format clean.
…xpressions (#128) ConditionLexer only recognized double-quoted strings, unlike the deleted legacy inline {{(...)}} parser (BooleanExpressionParser) which accepted both quote styles. Templates like {{(Status = 'Active')}} silently broke. Add an opt-in `allowSingleQuotedStrings` flag to ConditionLexer (default false, preserving current behavior). PlaceholderVisitor's inline {{(...)}} path now constructs the lexer with the flag on; ConditionalEvaluator's {{#if}}/text/standalone paths keep constructing it with the flag off, so that behavior is unchanged byte-for-byte. Refs #128, #129
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.
Closes #128.
What & why
Replaces the two separate condition engines (
Conditionals/ConditionalEvaluatorfor{{#if}}/text/standalone, andExpressions/BooleanExpressionParserfor inline{{(...)}}) with one extensible engine — lexer → Pratt parser → AST → operator registry → evaluator — underTriasDev.Templify/Conditionals/Engine/. Adding a future operator now means registering a singleIConditionOperator; the parser is never touched.New operators & syntax
in(Role in Roles,Status in ("Active","Pending"),Status in "A,B"); negation vianot(not Role in Roles).contains,startswith,endswith(case-sensitive / ordinal, consistent with=).exists,is empty,is not empty(postfix).exists= variable present regardless of value;is empty= null / empty-or-whitespace string / empty collection (missing var is also empty).(A or B) and C.All new operators work in both
{{#if}}conditionals and inline{{(...)}}expressions.Architecture
andbinds tighter thanor; parentheses override.ConditionDialectsupplies the two externally-observable semantic profiles: Default ({{#if}}, text templates, standaloneIConditionEvaluator— rich truthiness, numeric comparison) and Inline ({{(...)}}— bool-only truthiness,IComparablecomparison). New-operator element equality is dialect-independent (ordinal, viaConditionValueOps).IConditionEvaluator/ConditionEvaluator/IConditionContext/ConditionContext).Validate()is now parser-based (spec §8) with pre-checks for empty/unbalanced-quotes and a mapping layer that preserves the existing typed validation issues.Expressions/engine removed.Compatibility
External compatibility was the hard requirement. The entire pre-existing test suite passes unchanged (the only removed test file,
BooleanExpressionParserTests, is re-covered by newInlineDialectTests). All 28ConditionValidationTestspass unedited.A few narrow, previously-untested edge behaviors changed and were explicitly accepted and locked with characterization tests + a docs note ("Reserved words & literal quoting"):
{{(A = B)}}compares two variables).>=/<=on incomparable operands →false(wastrue).= "empty", not= empty).Testing & docs
dotnet formatclean.for-template-authors/conditionals.md,boolean-expressions.md,template-syntax.md,FAQ.md,for-developers/condition-evaluation.md, and theIConditionEvaluatorXML docs.Design spec:
docs/superpowers/specs/2026-07-20-extensible-condition-operators-design.md· Plan:docs/superpowers/plans/2026-07-20-extensible-condition-operators.md🤖 Built with subagent-driven development (fresh implementer + spec/quality review per task, plus a final whole-branch review).