fix(policy): support explicit policy_id registration

[sahiee-dev]

support explicit policy_id registration

• Update PolicyEngine.register to use policy.policy_id
• Deprecate brittle name-to-id string conversion
• Ensure policies like GDPR are correctly loaded

Summary by CodeRabbit

• Bug Fixes

  • Improved policy identification and validation to ensure policies are properly tracked and matched with their configurations.
  • Enhanced error handling for policy configuration mismatches.

• Improvements

  • Refined policy logging to include identifiers for better traceability and debugging.

[coderabbitai]

📝 Walkthrough

Walkthrough

Policy registration and identification mechanism updated to use policy hashing based on normalized code and canonical configuration instead of source-based hashing. PolicyDescriptor now stores policy_hash. PolicyEngine.register uses policy_id for config lookup with name-based fallback, and compute_policy_hash performs SHA-256 hashing of normalized source and canonical config.

Changes

Cohort / File(s)

Summary

Policy Hashing Refactor backend/app/services/policy/engine.py

Replaced source-based hashing with policy hashing. PolicyDescriptor field changed from source_hash to policy_hash. Removed compute_source_hash() from Policy. Added compute_policy_hash() to PolicyEngine that normalizes policy source via AST, requires policy_id, retrieves canonical config, and computes SHA-256 hash. Updated PolicyEngine.register() to use policy_id for config lookup (with fallback to normalized name) and modified logging to include policy_id. Policy identification now reflects both normalized code structure and canonical config.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hashing policies with care and grace,
We normalize the source, embrace the case,
No more by names, but ids we bind,
With config canonized, stability we find,
Hash by code and config wise,
A stronger policy in disguise! 🔐

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately summarizes the main change: introducing explicit policy_id registration support in the policy engine, which is the core objective described in the PR.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/policy-engine-registration

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

backend/app/services/policy/engine.py (1)

208-213: Reuse the cached descriptor hash during evaluation.

policy_set() already computes and freezes each policy_hash. Recomputing it here repeats the expensive source normalization on every batch and can drift from the cached PolicySet snapshot if the engine ever outlives a code reload.

♻️ Proposed refactor

-        for policy in self._policies:
-            policy_hash = self.compute_policy_hash(policy)
-
+        for policy, descriptor in zip(self._policies, ps.policies):
             try:
                 violations = policy.evaluate(
                     events=events,
                     policy_version=ps.version,
-                    policy_hash=policy_hash,
+                    policy_hash=descriptor.policy_hash,
                 )

Also applies to: 279-289

🤖 Prompt for AI Agents

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

In `@backend/app/services/policy/engine.py` around lines 208 - 213, The code
recomputes policy hashes during evaluation (calling self.compute_policy_hash(p))
even though policy_set() already computed and froze each policy_hash; replace
those recomputations with the cached value (use the Policy object's policy_hash
field or the frozen PolicyDescriptor) in the block constructing PolicyDescriptor
(and the similar block at 279-289) so you read p.policy_hash (or the existing
descriptor.policy_hash) instead of calling compute_policy_hash; ensure you
remove the expensive normalization call and rely on the precomputed hash to
avoid drift across reloads.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@backend/app/services/policy/engine.py`:
- Around line 181-196: Ensure Policy declares a mandatory policy_id and validate
it once during registration: in Policy (class/dataclass) add policy_id: str,
then in PolicyEngine.register (the block that previously accessed
policy.policy_id/name) first check that policy.policy_id exists and is a
non-empty string, look up that exact key in self._config["policies"] and if
missing raise an explicit error; store the resolved policy_id and its
policy_config in local variables and use those for deciding enabled and
appending to self._policies (instead of redoing a name-based fallback that
dereferences policy.policy_id earlier). Finally, update compute_policy_hash to
use the same resolved policy_id and the stored/validated policy_config (from the
same config lookup) when computing the hash so the recorded hash matches the
config actually used.

---

Nitpick comments:
In `@backend/app/services/policy/engine.py`:
- Around line 208-213: The code recomputes policy hashes during evaluation
(calling self.compute_policy_hash(p)) even though policy_set() already computed
and froze each policy_hash; replace those recomputations with the cached value
(use the Policy object's policy_hash field or the frozen PolicyDescriptor) in
the block constructing PolicyDescriptor (and the similar block at 279-289) so
you read p.policy_hash (or the existing descriptor.policy_hash) instead of
calling compute_policy_hash; ensure you remove the expensive normalization call
and rely on the precomputed hash to avoid drift across reloads.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 12851568-565b-4eac-9402-9b1ae28e9d62

📥 Commits

Reviewing files that changed from the base of the PR and between 22a4387 and 35271b7.

📒 Files selected for processing (1)

• backend/app/services/policy/engine.py