Skip to content

Conversation

@khvn26
Copy link
Member

@khvn26 khvn26 commented Nov 7, 2025

Closes #166.

This is the first in a series of 3 PRs:

  1. New engine implementation
  2. Mapper layer
  3. SDK changes, old engine removal

In this PR, we:

  • Add tooling for code generation from the EvaluationContext schema. Note that the resulting classes have to be modified to support generic metadata types.
  • Add temporary tooling to circumvent the submodule restrictions and be able to run tests against different revisions of engine-test-data. This will be removed in PR 3/3.
  • Implement a new ContextEvaluator engine that takes JSONPath-based Context Values into account, covered by latest engine-test-data. The type-coerced evaluation part is reused from the old (current) engine implementation and will be migrated in PR 3/3.
  • Bump xunit to 2.4.2 to take advantage of Assert.Equivalent.
  • Run tests against .NET 10.x.
  • Skip the flaky test — see TestAnalyticsDataConsistencyWithConcurrentCallsToGetFlags is flaky #170.

@khvn26 khvn26 requested a review from a team as a code owner November 7, 2025 12:48
@khvn26 khvn26 requested review from gagantrivedi and removed request for a team November 7, 2025 12:48
@khvn26 khvn26 requested a review from emyller November 7, 2025 13:03
Copy link
Contributor

@emyller emyller left a comment

Choose a reason for hiding this comment

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

The engine code makes sense! There are only a few minor points I think need visiting.

@gagantrivedi gagantrivedi requested a review from Copilot November 11, 2025 04:06
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 PR implements the first phase of a new engine evaluation system that supports Context Values with JSONPath-based evaluation. It introduces GetEvaluationResult method and the ContextEvaluator engine, while maintaining compatibility with the existing engine implementation.

Key changes:

  • New ContextEvaluator engine with Context Values support via JSONPath expressions
  • Auto-generated data models from JSON Schema for EvaluationContext and EvaluationResult
  • Temporary infrastructure for testing against engine-test-data revisions outside the submodule

Reviewed Changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Makefile Adds build tooling for code generation from JSON schemas and temporary engine test data cloning
Flagsmith.EngineTest/EngineTestV2.cs New test suite for the Context Values engine using engine-test-data v2
Flagsmith.EngineTest/EngineTestData Updates submodule commit reference
Flagsmith.EngineTest/EngineTest.csproj Bumps xunit to 2.4.2 for Assert.Equivalent support
Flagsmith.Engine/Segment/Evaluator.cs Implements ContextEvaluator with JSONPath support and improves type conversion safety
Flagsmith.Engine/Segment/Constants.cs Adds DefaultPriority constant for segment override priority handling
Flagsmith.Engine/Interfaces/IEngine.cs Adds GetEvaluationResult interface method
Flagsmith.Engine/EvaluationResult/EvaluationResult.cs Auto-generated result classes from JSON Schema
Flagsmith.Engine/EvaluationContext/EvaluationContext.cs Auto-generated context classes from JSON Schema
Flagsmith.Engine/Engine.cs Implements GetEvaluationResult and Identity.Key enrichment
Flagsmith.Client.Test/FlagsmithTest.cs Skips flaky analytics test
Flagsmith.Client.Test/Fixtures.cs Code formatting cleanup
.gitmodules Changes submodule branch reference to tag
.gitignore Adds EngineTestDataV2 to ignored paths
.github/workflows/formatting-and-tests.yml Adds .NET 10.x testing and engine-test-data v2 cloning step

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

khvn26 and others added 6 commits November 11, 2025 11:06
Co-authored-by: Evandro Myller <22429+emyller@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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

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


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

Copy link
Contributor

@emyller emyller left a comment

Choose a reason for hiding this comment

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

Only pending a non-blocking question here. Otherwise LGTM.

@khvn26 khvn26 merged commit b064127 into main Nov 11, 2025
20 of 21 checks passed
@khvn26 khvn26 deleted the feat/context-engine branch November 11, 2025 18:25
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.

Create a new engine to implement context values

4 participants