Eng 2195 be policy evaluation engine #7253

JadeCara · 2026-01-27T00:42:49Z

Description Of Changes

🎯 Create a Policy Evaluation Engine for Fides

This PR creates a new policy evaluation engine to accept a PrivacyRequest object transform it to evaluation data dictionary then compare to policies with conditions. For each policy, check if conditions match and return the matching policy + evaluation result. There should also be a fallback to default if no policies match. There is a check in place when creating a condition to validate there are no identical conditions. So there should be 1 or 0 matches.

Code Changes

src/fides/api/task/conditional_dependencies/policy_evaluation.py - new engine
src/fides/api/task/conditional_dependencies/util.py added field address extraction function
src/fides/api/task/manual/manual_task_conditional_evaluation.py updated to import new function
src/fides/api/task/manual/manual_task_utils.py updated to use new function
tests/api/task/conditional_dependencies/test_policy_evaluation.py tests!

Steps to Confirm

This is not live yet. All tests should pass.

Pre-Merge Checklist

vercel · 2026-01-27T00:42:54Z

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ignored	Preview	Jan 30, 2026 1:49am
fides-privacy-center	Ignored		Jan 30, 2026 1:49am

greptile-apps · 2026-01-27T01:20:23Z

Greptile Overview

Greptile Summary

Created a new policy evaluation engine that evaluates PrivacyRequest objects against policies with conditions. The engine transforms the privacy request into evaluation data, compares against policy conditions, and returns the first matching policy or falls back to a default policy based on action type.

Key Changes:

New PolicyEvaluator class handles policy condition evaluation with proper eager loading to avoid N+1 queries
Moved extract_field_addresses function from manual_task_utils.py to shared util.py module to support both manual tasks and policy evaluation
Added PolicyEvaluationResult schema to encapsulate evaluation results
Comprehensive test coverage for policy routing, default fallback, error handling, and edge cases

Issues Found:

The query in _query_policy_conditions lacks explicit ordering, which could lead to non-deterministic behavior when multiple policies match (though the PR description mentions there should be a validation check preventing identical conditions)
Broad exception handling uses logger.exception() which logs details but may obscure debugging in some scenarios

Confidence Score: 4/5

Safe to merge with one logical concern about non-deterministic policy ordering
Well-structured implementation with good test coverage and proper separation of concerns. The main concern is the lack of explicit ordering in policy queries which could cause non-deterministic behavior, though the PR description mentions validation prevents identical conditions. The broad exception handling is acceptable for graceful degradation to default policies.
Pay attention to src/fides/api/task/conditional_dependencies/policy_evaluation.py - verify that policy ordering behavior matches expectations

Important Files Changed

Filename	Overview
src/fides/api/task/conditional_dependencies/policy_evaluation.py	New policy evaluation engine with good structure, minor ordering concern
src/fides/api/task/conditional_dependencies/util.py	Moved field address extraction function, handles both Pydantic and dict formats
src/fides/api/task/conditional_dependencies/schemas.py	Added PolicyEvaluationResult schema with appropriate configuration
tests/api/task/conditional_dependencies/test_policy_evaluation.py	Comprehensive test coverage for policy routing, defaults, and error cases

greptile-apps

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

src/fides/api/task/conditional_dependencies/policy_evaluation.py

changelog/7253.yaml

JadeCara · 2026-01-27T21:57:06Z

@greptile please review

greptile-apps

_{4 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

src/fides/api/task/conditional_dependencies/policy_evaluation.py

tests/api/task/conditional_dependencies/test_policy_evaluation.py

…thyca/fides into ENG-2195-be-policy-evaluation-engine

JadeCara · 2026-01-27T22:09:55Z

@greptile please review

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

tests/api/task/conditional_dependencies/test_policy_evaluation.py

erosselli · 2026-01-29T12:37:57Z

src/fides/api/task/conditional_dependencies/policy_evaluation.py

+            logger.error(
+                f"Error evaluating policy {policy_condition.policy.key}: "
+                f"{type(exc).__name__}: {exc}",
+                exc_info=True,
+            )


is this equivalent to using logger.exception ? will this include a stack trace in dev envs? (and what about prod ones?)

Updated to use exception

erosselli · 2026-01-29T12:38:22Z

src/fides/api/task/conditional_dependencies/policy_evaluation.py

+        # Default to access if no action_type provided
+        return _get_default_policy_result(
+            db, action_type or ActionType.access
+        )


I have little context on the policy conditions, but why is access the default?

Updating to make Action type not optional :)

erosselli · 2026-01-29T12:40:25Z

src/fides/api/task/conditional_dependencies/policy_evaluation.py

+            root_condition = ConditionTypeAdapter.validate_python(
+                policy_condition.condition_tree
+            )


I'm slightly confused as to what this is doing here, I thought now we had the entire condition tree on the root node?

I updated the name to be more clear. This is loading the json version of a condition tree into the Condition schema.

erosselli · 2026-01-29T12:41:30Z

src/fides/api/task/conditional_dependencies/policy_evaluation.py

+    return PolicyEvaluationResult(
+        policy=policy, evaluation_result=None, is_default=True
+    )


I don't follow why we need the "default policy" if we're just returning None for the result ? can't we just leave policy as None as well ?

This is evaluating conditions to select a policy - so we need to return a policy. The evaluation result is the set of conditions which matched for logging purposes.

For defaults there was no set of conditions that matched, its just the default (fallback)

erosselli · 2026-01-29T12:42:36Z

src/fides/api/task/conditional_dependencies/policy_evaluation.py

+            db, action_type or ActionType.access
+        )
+
+    evaluator = ConditionEvaluator(db)


what do you think of having a PolicyEvaluator class that contains a evaluate_policy_conditions method , instead of just having it be a function?

I have no issue with that, refactoring to make it a class. :)

erosselli · 2026-01-29T12:46:26Z

tests/api/task/conditional_dependencies/test_policy_evaluation.py

+        assert result.policy.key == "conditional"
+
+
+class TestEdgeCases:


nit: I don't love "TestEdgeCases" as a test suite name hah , arguably both the tests in this class can be moved into TestDefaultFallback ?

JadeCara · 2026-01-30T02:13:17Z

@greptile please review

greptile-apps

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-30T02:16:35Z

src/fides/api/task/conditional_dependencies/policy_evaluation.py

+        query = (
+            self.db.query(PolicyCondition)
+            .join(Policy, PolicyCondition.policy_id == Policy.id)
+            .options(contains_eager(PolicyCondition.policy))
+            .join(Rule, Policy.id == Rule.policy_id)
+            .filter(Rule.action_type == action_type)
+        )
+        return query.all()


query doesn't specify ordering - when multiple policies match, the result is non-deterministic

add explicit ordering (e.g. by created_at or a dedicated priority field) to ensure consistent policy selection

There is a check to prevent multiple policies from having the same conditions. There should be only 1 match.

We could return query.one() so if at any point this somehow returns multiple ones, we get a failure :)

src/fides/api/task/conditional_dependencies/policy_evaluation.py

erosselli · 2026-01-30T12:57:54Z

tests/conftest.py

 @pytest.mark.asyncio
 @pytest.fixture(scope="function")
-def seed_data(session):
+def seed_data(db: Session):


is this change related?

I wanted to use seed data to get all of the premade policies but when I used this fixture I got an error saying there was no session fixture. Pretty sure no other tests were using this or we would have seen a lot more errors!

erosselli

Approved, left two comments

Jade Wibbels added 3 commits January 26, 2026 17:28

.

3deae48

clean up

2ec3e01

clean up

1b87d63

JadeCara marked this pull request as ready for review January 27, 2026 01:18

JadeCara requested a review from a team as a code owner January 27, 2026 01:18

JadeCara requested review from erosselli and removed request for a team January 27, 2026 01:18

greptile-apps bot reviewed Jan 27, 2026

View reviewed changes

src/fides/api/task/conditional_dependencies/policy_evaluation.py Outdated Show resolved Hide resolved

changelog

4838e9e

JadeCara commented Jan 27, 2026

View reviewed changes

changelog/7253.yaml Outdated Show resolved Hide resolved

JadeCara and others added 5 commits January 26, 2026 18:34

Apply suggestion from @JadeCara

c982cc4

.

b3ba471

removed N+1 query

915dce6

updated removing vaggue priority

c16c3e7

Merge branch 'main' into ENG-2195-be-policy-evaluation-engine

debc23b

greptile-apps bot reviewed Jan 27, 2026

View reviewed changes

vercel bot deployed to Preview – fides-plus-nightly January 27, 2026 22:01 View deployment

Jade Wibbels added 4 commits January 27, 2026 15:03

update query

2008462

t Merge branch 'ENG-2195-be-policy-evaluation-engine' of github.com:e…

7221b0a

…thyca/fides into ENG-2195-be-policy-evaluation-engine

update test description

29fbbb6

improved error handling

b9de23b

greptile-apps bot reviewed Jan 27, 2026

View reviewed changes

Merge branch 'main' into ENG-2195-be-policy-evaluation-engine

6066569

vercel bot deployed to Preview – fides-plus-nightly January 28, 2026 16:55 View deployment

Jade Wibbels added 2 commits January 28, 2026 13:26

cleanup default policy query

d0fc708

cleanup default policy query

4dbc93a

cleanup default policy query

06213c8

JadeCara commented Jan 28, 2026

View reviewed changes

tests/api/task/conditional_dependencies/test_policy_evaluation.py Outdated Show resolved Hide resolved

JadeCara added 2 commits January 28, 2026 15:27

Apply suggestion from @JadeCara

bb8d51b

Merge branch 'main' into ENG-2195-be-policy-evaluation-engine

1d70c17

vercel bot deployed to Preview – fides-plus-nightly January 28, 2026 22:35 View deployment

Merge branch 'main' into ENG-2195-be-policy-evaluation-engine

73374dc

vercel bot deployed to Preview – fides-plus-nightly January 28, 2026 23:36 View deployment

erosselli reviewed Jan 29, 2026

View reviewed changes

Merge branch 'main' into ENG-2195-be-policy-evaluation-engine

e927d6a

vercel bot deployed to Preview – fides-plus-nightly January 29, 2026 23:55 View deployment

update for review

7327b3e

JadeCara requested a review from erosselli January 30, 2026 01:49

greptile-apps bot reviewed Jan 30, 2026

View reviewed changes

erosselli reviewed Jan 30, 2026

View reviewed changes

erosselli approved these changes Jan 30, 2026

View reviewed changes

		assert result.policy.key == "conditional"


		class TestEdgeCases:

Eng 2195 be policy evaluation engine #7253

Are you sure you want to change the base?

Eng 2195 be policy evaluation engine #7253

Uh oh!

Conversation

JadeCara commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

Uh oh!

vercel bot commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Overview

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JadeCara commented Jan 27, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JadeCara commented Jan 27, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JadeCara commented Jan 30, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

JadeCara commented Jan 27, 2026 •

edited

Loading

vercel bot commented Jan 27, 2026 •

edited

Loading

greptile-apps bot commented Jan 27, 2026 •

edited

Loading