Repository: cedar-policy/cedar
Pull request #1520 (2025-03-04 14:27:47)

remove Serialize and Deserialize from many core/validator types
## Description of changes

Removed `Serialize` and `Deserialize` from many core/validator types.  This serialization format was undocumented, unstable, and used only by DRT (until https://github.com/cedar-policy/cedar-spec/pull/562 and https://github.com/cedar-policy/cedar-spec/pull/566 remove the last use of it in DRT).  We will still have `Serialize` and `Deserialize` for officially-supported formats, including in the `entities::json` module of Core, the `est` module of Core, etc; and we also have the ability (in `cedar-policy`) to ser/de via Protobuf for many of these types as well.

Specifically, this PR removes `Serialize` and `Deserialize` from the following core and validator structures (note none of these are publicly exported in `cedar-policy`):
- Expr-related structures: `Expr`, `ExprKind`, `RestrictedExpr`, `BorrowedRestrictedExpr`, `Literal`, `UnaryOp`, `BinaryOp`, `Pattern`, `PatternElem`, `Unknown`, `Value`, `ValueKind` (note those last two previously serialized as `Expr`)
- Policy-related structures: `PolicySet`, `LiteralPolicySet`, `Template`, `StaticPolicy`, `LiteralPolicy`, `TemplateBody`, `Annotations`, `PrincipalConstraint`, `ResourceConstraint`, `PrincipalOrResourceConstraint`, `ActionConstraint`, `PrincipalOrResource`, `EntityReference`
- Request-related structures: `Request`, `EntityUIDEntry`, `Context`
- Entity-related structures: `Entities`, `Entity`
- Type-related structures: `Type`
- Parser-related structures: `Node`, `Loc`
- Validator-related structures: `ValidatorSchema`, `ValidatorActionId`, `ValidatorApplySpec`

In addition, this PR removes entirely the following structures:
- Core's `PartialValueSeriailzedAsExpr`, in favor of simply using `PartialValue` directly
- Core's `BorrowedLiteralPolicy`, as no longer used

## Issue #, if available

## Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

- [x] A change "invisible" to users (e.g., documentation, changes to "internal" crates like `cedar-policy-core`, `cedar-validator`, etc.)

I confirm that this PR (choose one, and delete the other options):

- [x] Does not update the CHANGELOG because my change does not significantly impact released code.

I confirm that [`cedar-spec`](https://github.com/cedar-policy/cedar-spec) (choose one, and delete the other options):

- [x] Requires updates, and I have made / will make these updates myself. (Please include in your description a timeline or link to the relevant PR in `cedar-spec`, and how you have tested that your updates are correct.)

I confirm that [`docs.cedarpolicy.com`](https://docs.cedarpolicy.com/) (choose one, and delete the other options):

- [x] Does not require updates because my change does not impact the Cedar language specification.


github-actions[bot]: 
# Coverage Report
Head Commit: ```2cbbdb7dc2d71284a3f168703073c17c8676c6b0```

Base Commit: ```7f40eb6ac47082acf1ecfaf34c5084f9351fa0ea```

[Download the full coverage report.](https://github.com/cedar-policy/cedar/actions/runs/13655997635/artifacts/2689341836)

## Coverage of Added or Modified Lines of Rust Code
**Required coverage:** 80.00%

**Actual coverage:** 72.09%

**Status:** FAILED :x:

<details>
<summary><b>Details</b></summary>

| File | Status | Covered | Coverage | Missed Lines |
|:-----|:------:|:-------:|---------:|:-------------|
cedar-policy-core/src/ast/entity.rs | :green_circle: | 13/13 | 100.00% | 
cedar-policy-core/src/ast/expr.rs | :green_circle: | 1/1 | 100.00% | 
cedar-policy-core/src/ast/policy.rs | :red_circle: | 0/2 | 0.00% | 889, 1316
cedar-policy-core/src/ast/value.rs | :green_circle: | 1/1 | 100.00% | 
cedar-policy-core/src/parser/node.rs | :red_circle: | 0/1 | 0.00% | 26
cedar-policy-validator/src/schema.rs | :green_circle: | 2/2 | 100.00% | 
cedar-policy-validator/src/schema/action.rs | :red_circle: | 0/2 | 0.00% | 75, 148
cedar-policy-validator/src/schema/namespace_def.rs | :green_circle: | 3/3 | 100.00% | 
cedar-policy/src/proto/ast.rs | :red_circle: | 11/18 | 61.11% | 143-149
</details>

## Coverage of All Lines of Rust Code
**Required coverage:** 80.00%

**Actual coverage:** 80.65%

**Status:** PASSED :white_check_mark:

<details>
<summary><b>Details</b></summary>

| Package | Status | Covered | Coverage | Base Coverage |
|:--------|:------:|:-------:|---------:|--------------:|
cedar-policy | :red_circle: | 3059/4392 | 69.65% | 69.70%
cedar-policy-cli | :red_circle: | 539/927 | 58.14% | 58.14%
cedar-policy-core | :green_circle: | 11161/13613 | 81.99% | 82.10%
cedar-policy-formatter | :green_circle: | 914/1043 | 87.63% | 87.63%
cedar-policy-validator | :green_circle: | 8242/9647 | 85.44% | 85.44%
cedar-wasm | :red_circle: | 0/29 | 0.00% | 0.00%
</details>


Patch:
diff --git a/cedar-policy-core/src/ast/annotation.rs b/cedar-policy-core/src/ast/annotation.rs
index ab98f64c..b3fd4e0d 100644
--- a/cedar-policy-core/src/ast/annotation.rs
+++ b/cedar-policy-core/src/ast/annotation.rs
@@ -25,14 +25,9 @@ use crate::parser::Loc;
 use super::AnyId;
 
 /// Struct which holds the annotations for a policy
-#[derive(Serialize, Deserialize, Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
+#[derive(Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
 #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
-pub struct Annotations(
-    #[serde(default)]
-    #[serde(skip_serializing_if = "BTreeMap::is_empty")]
-    #[serde(with = "::serde_with::rust::maps_duplicate_key_is_error")]
-    BTreeMap<AnyId, Annotation>,
-);
+pub struct Annotations(BTreeMap<AnyId, Annotation>);
 
 impl std::fmt::Display for Annotations {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
diff --git a/cedar-policy-core/src/ast/entity.rs b/cedar-policy-core/src/ast/entity.rs
index 39988d50..22b83822 100644
--- a/cedar-policy-core/src/ast/entity.rs
+++ b/cedar-policy-core/src/ast/entity.rs
@@ -26,7 +26,6 @@ use educe::Educe;
 use itertools::Itertools;
 use miette::Diagnostic;
 use serde::{de::Deserializer, ser::Serializer, Deserialize, Serialize};
-use serde_with::{serde_as, TryFromInto};
 use smol_str::SmolStr;
 use std::collections::{BTreeMap, HashMap, HashSet};
 use std::str::FromStr;
@@ -440,17 +439,15 @@ impl<'a> arbitrary::Arbitrary<'a> for Eid {
 }
 
 /// Entity datatype
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone)]
 pub struct Entity {
     /// UID
     uid: EntityUID,
 
-    /// Internal BTreeMap of attributes.
-    /// We use a btreemap so that the keys have a deterministic order.
+    /// Internal `BTreeMap` of attributes.
     ///
-    /// In the serialized form of `Entity`, attribute values appear as
-    /// `RestrictedExpr`s, for mostly historical reasons.
-    attrs: BTreeMap<SmolStr, PartialValueSerializedAsExpr>,
+    /// We use a `BTreeMap` so that the keys have a deterministic order.
+    attrs: BTreeMap<SmolStr, PartialValue>,
 
     /// Set of indirect ancestors of this `Entity` as UIDs
     indirect_ancestors: HashSet<EntityUID>,
@@ -466,10 +463,7 @@ pub struct Entity {
     ///
     /// Like for `attrs`, we use a `BTreeMap` so that the tags have a
     /// deterministic order.
-    /// And like in `attrs`, the values in `tags` appear as `RestrictedExpr` in
-    /// the serialized form of `Entity`.
-    #[serde(skip_serializing_if = "BTreeMap::is_empty")]
-    tags: BTreeMap<SmolStr, PartialValueSerializedAsExpr>,
+    tags: BTreeMap<SmolStr, PartialValue>,
 }
 
 impl std::hash::Hash for Entity {
@@ -501,7 +495,7 @@ impl Entity {
                     was_attr,
                     err,
                 })?;
-            Ok((k, attr_val.into()))
+            Ok((k, attr_val))
         };
         let evaluated_attrs = attrs
             .into_iter()
@@ -535,32 +529,12 @@ impl Entity {
         parents: HashSet<EntityUID>,
         tags: impl IntoIterator<Item = (SmolStr, PartialValue)>,
     ) -> Self {
-        Self::new_with_attr_partial_value_serialized_as_expr(
-            uid,
-            attrs.into_iter().map(|(k, v)| (k, v.into())).collect(),
-            indirect_ancestors,
-            parents,
-            tags.into_iter().map(|(k, v)| (k, v.into())).collect(),
-        )
-    }
-
-    /// Create a new `Entity` with this UID, attributes, ancestors, and tags
-    ///
-    /// Unlike in `Entity::new()`, in this constructor, attributes and tags are
-    /// expressed as `PartialValueSerializedAsExpr`.
-    pub fn new_with_attr_partial_value_serialized_as_expr(
-        uid: EntityUID,
-        attrs: BTreeMap<SmolStr, PartialValueSerializedAsExpr>,
-        indirect_ancestors: HashSet<EntityUID>,
-        parents: HashSet<EntityUID>,
-        tags: BTreeMap<SmolStr, PartialValueSerializedAsExpr>,
-    ) -> Self {
-        Entity {
+        Self {
             uid,
-            attrs,
+            attrs: attrs.into_iter().collect(),
             indirect_ancestors,
             parents,
-            tags,
+            tags: tags.into_iter().collect(),
         }
     }
 
@@ -571,12 +545,12 @@ impl Entity {
 
     /// Get the value for the given attribute, or `None` if not present
     pub fn get(&self, attr: &str) -> Option<&PartialValue> {
-        self.attrs.get(attr).map(|v| v.as_ref())
+        self.attrs.get(attr)
     }
 
     /// Get the value for the given tag, or `None` if not present
     pub fn get_tag(&self, tag: &str) -> Option<&PartialValue> {
-        self.tags.get(tag).map(|v| v.as_ref())
+        self.tags.get(tag)
     }
 
     /// Is this `Entity` a (direct or indirect) descendant of `e` in the entity hierarchy?
@@ -631,12 +605,12 @@ impl Entity {
 
     /// Iterate over this entity's attributes
     pub fn attrs(&self) -> impl Iterator<Item = (&SmolStr, &PartialValue)> {
-        self.attrs.iter().map(|(k, v)| (k, v.as_ref()))
+        self.attrs.iter()
     }
 
     /// Iterate over this entity's tags
     pub fn tags(&self) -> impl Iterator<Item = (&SmolStr, &PartialValue)> {
-        self.tags.iter().map(|(k, v)| (k, v.as_ref()))
+        self.tags.iter()
     }
 
     /// Create an `Entity` with the given UID, no attributes, no parents, and no tags.
@@ -713,19 +687,12 @@ impl Entity {
         HashSet<EntityUID>,
         HashMap<SmolStr, PartialValue>,
     ) {
-        let Self {
-            uid,
-            attrs,
-            indirect_ancestors,
-            parents,
-            tags,
-        } = self;
         (
-            uid,
-            attrs.into_iter().map(|(k, v)| (k, v.0)).collect(),
-            indirect_ancestors,
-            parents,
-            tags.into_iter().map(|(k, v)| (k, v.0)).collect(),
+            self.uid,
+            self.attrs.into_iter().collect(),
+            self.indirect_ancestors,
+            self.parents,
+            self.tags.into_iter().collect(),
         )
     }
 
@@ -818,47 +785,6 @@ impl std::fmt::Display for Entity {
     }
 }
 
-/// `PartialValue`, but serialized as a `RestrictedExpr`.
-///
-/// (Extension values can't be directly serialized, but can be serialized as
-/// `RestrictedExpr`)
-#[serde_as]
-#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
-pub struct PartialValueSerializedAsExpr(
-    #[serde_as(as = "TryFromInto<RestrictedExpr>")] PartialValue,
-);
-
-impl AsRef<PartialValue> for PartialValueSerializedAsExpr {
-    fn as_ref(&self) -> &PartialValue {
-        &self.0
-    }
-}
-
-impl std::ops::Deref for PartialValueSerializedAsExpr {
-    type Target = PartialValue;
-    fn deref(&self) -> &Self::Target {
-        &self.0
-    }
-}
-
-impl From<PartialValue> for PartialValueSerializedAsExpr {
-    fn from(value: PartialValue) -> PartialValueSerializedAsExpr {
-        PartialValueSerializedAsExpr(value)
-    }
-}
-
-impl From<PartialValueSerializedAsExpr> for PartialValue {
-    fn from(value: PartialValueSerializedAsExpr) -> PartialValue {
-        value.0
-    }
-}
-
-impl std::fmt::Display for PartialValueSerializedAsExpr {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.0)
-    }
-}
-
 /// Error type for evaluation errors when evaluating an entity attribute or tag.
 /// Contains some extra contextual information and the underlying
 /// `EvaluationError`.
diff --git a/cedar-policy-core/src/ast/expr.rs b/cedar-policy-core/src/ast/expr.rs
index 348ebb97..4ee9043f 100644
--- a/cedar-policy-core/src/ast/expr.rs
+++ b/cedar-policy-core/src/ast/expr.rs
@@ -44,7 +44,7 @@ extern crate tsify;
 /// where the expression was written in policy source code, and some generic
 /// data which is stored on each node of the AST.
 /// Cloning is O(1).
-#[derive(Educe, Serialize, Deserialize, Debug, Clone)]
+#[derive(Educe, Debug, Clone)]
 #[educe(PartialEq, Eq, Hash)]
 pub struct Expr<T = ()> {
     expr_kind: ExprKind<T>,
@@ -56,7 +56,7 @@ pub struct Expr<T = ()> {
 
 /// The possible expression variants. This enum should be matched on by code
 /// recursively traversing the AST.
-#[derive(Serialize, Deserialize, Hash, Debug, Clone, PartialEq, Eq)]
+#[derive(Hash, Debug, Clone, PartialEq, Eq)]
 pub enum ExprKind<T = ()> {
     /// Literal value
     Lit(Literal),
@@ -819,7 +819,7 @@ pub enum SubstitutionError {
 }
 
 /// Representation of a partial-evaluation Unknown at the AST level
-#[derive(Serialize, Deserialize, Hash, Debug, Clone, PartialEq, Eq)]
+#[derive(Hash, Debug, Clone, PartialEq, Eq)]
 pub struct Unknown {
     /// The name of the unknown
     pub name: SmolStr,
diff --git a/cedar-policy-core/src/ast/literal.rs b/cedar-policy-core/src/ast/literal.rs
index c0d27c81..26f1737e 100644
--- a/cedar-policy-core/src/ast/literal.rs
+++ b/cedar-policy-core/src/ast/literal.rs
@@ -16,7 +16,6 @@
 
 use crate::ast::{EntityUID, Integer, StaticallyTyped, Type};
 use crate::parser;
-use serde::{Deserialize, Serialize};
 use smol_str::SmolStr;
 use std::sync::Arc;
 
@@ -32,7 +31,7 @@ use std::sync::Arc;
 /// `Expr::Set`, not `Expr::Lit`.
 ///
 /// Cloning is O(1).
-#[derive(Serialize, Deserialize, Hash, Debug, PartialEq, Eq, Clone, PartialOrd, Ord)]
+#[derive(Hash, Debug, PartialEq, Eq, Clone, PartialOrd, Ord)]
 pub enum Literal {
     /// Boolean value
     Bool(bool),
diff --git a/cedar-policy-core/src/ast/ops.rs b/cedar-policy-core/src/ast/ops.rs
index f40c34ec..442d4683 100644
--- a/cedar-policy-core/src/ast/ops.rs
+++ b/cedar-policy-core/src/ast/ops.rs
@@ -14,10 +14,8 @@
  * limitations under the License.
  */
 
-use serde::{Deserialize, Serialize};
-
 /// Built-in operators with exactly one argument
-#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Copy, Hash)]
+#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]
 #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub enum UnaryOp {
     /// Logical negation
@@ -45,7 +43,7 @@ impl std::fmt::Display for UnaryOp {
 }
 
 /// Built-in operators with exactly two arguments
-#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Copy, Hash)]
+#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]
 #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub enum BinaryOp {
     /// Equality
diff --git a/cedar-policy-core/src/ast/pattern.rs b/cedar-policy-core/src/ast/pattern.rs
index f4d1abbc..560947f3 100644
--- a/cedar-policy-core/src/ast/pattern.rs
+++ b/cedar-policy-core/src/ast/pattern.rs
@@ -16,10 +16,8 @@
 
 use std::sync::Arc;
 
-use serde::{Deserialize, Serialize};
-
 /// Represent an element in a pattern literal (the RHS of the like operation)
-#[derive(Deserialize, Serialize, Hash, Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Hash, Debug, Clone, Copy, PartialEq, Eq)]
 #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub enum PatternElem {
     /// A character literal
@@ -30,8 +28,7 @@ pub enum PatternElem {
 
 /// Represent a pattern literal (the RHS of the like operator)
 /// Also provides an implementation of the Display trait as well as a wildcard matching method.
-#[derive(Debug, Clone, Hash, Eq, PartialEq, Serialize, Deserialize)]
-#[serde(transparent)]
+#[derive(Debug, Clone, Hash, Eq, PartialEq)]
 pub struct Pattern {
     /// A vector of pattern elements
     elems: Arc<Vec<PatternElem>>,
diff --git a/cedar-policy-core/src/ast/policy.rs b/cedar-policy-core/src/ast/policy.rs
index 0cff6d10..5675a2cb 100644
--- a/cedar-policy-core/src/ast/policy.rs
+++ b/cedar-policy-core/src/ast/policy.rs
@@ -36,9 +36,7 @@ extern crate tsify;
 /// Contains both the AST for template, and the list of open slots in the template.
 ///
 /// Note that this "template" may have no slots, in which case this `Template` represents a static policy
-#[derive(Clone, Hash, Eq, PartialEq, Debug, Serialize, Deserialize)]
-#[serde(from = "TemplateBody")]
-#[serde(into = "TemplateBody")]
+#[derive(Clone, Hash, Eq, PartialEq, Debug)]
 pub struct Template {
     body: TemplateBody,
     /// INVARIANT (slot cache correctness): This Vec must contain _all_ of the open slots in `body`
@@ -556,9 +554,11 @@ pub type SlotEnv = HashMap<SlotId, EntityUID>;
 
 /// Represents either a static policy or a template linked policy.
 ///
-/// This is the serializable version because it simply refers to the `Template` by its Id
-/// and does not contain a reference to the `Template` itself
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+/// Contains less rich information than `Policy`. In particular, this form is
+/// easier to convert to/from the Protobuf representation of a `Policy`, because
+/// it simply refers to the `Template` by its Id and does not contain a
+/// reference to the `Template` itself.
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct LiteralPolicy {
     /// ID of the template this policy is an instance of
     template_id: PolicyID,
@@ -570,29 +570,6 @@ pub struct LiteralPolicy {
     values: SlotEnv,
 }
 
-/// A borrowed version of LiteralPolicy exclusively for serialization
-#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
-pub struct BorrowedLiteralPolicy<'a> {
-    /// ID of the template this policy is an instance of
-    template_id: &'a PolicyID,
-    /// ID of this link.
-    /// This is `None` for static policies, and the static policy ID is defined
-    /// as the `template_id`
-    link_id: Option<&'a PolicyID>,
-    /// Values of the slots
-    values: &'a SlotEnv,
-}
-
-impl<'a> From<&'a Policy> for BorrowedLiteralPolicy<'a> {
-    fn from(p: &'a Policy) -> Self {
-        Self {
-            template_id: p.template.id(),
-            link_id: p.link.as_ref(),
-            values: &p.values,
-        }
-    }
-}
-
 impl LiteralPolicy {
     /// Create a `LiteralPolicy` representing a static policy with the given ID.
     ///
@@ -762,7 +739,7 @@ impl From<Policy> for LiteralPolicy {
 /// Static Policies are policy that do not come from templates.
 /// They have the same structure as a template definition, but cannot contain slots
 // INVARIANT: (Static Policy Correctness): A Static Policy TemplateBody must have zero slots
-#[derive(Serialize, Deserialize, Clone, Hash, Eq, PartialEq, Debug)]
+#[derive(Clone, Hash, Eq, PartialEq, Debug)]
 pub struct StaticPolicy(TemplateBody);
 
 impl StaticPolicy {
@@ -909,7 +886,7 @@ impl From<StaticPolicy> for Arc<Template> {
 
 /// Policy datatype. This is used for both templates (in which case it contains
 /// slots) and static policies (in which case it contains zero slots).
-#[derive(Educe, Serialize, Deserialize, Clone, Debug)]
+#[derive(Educe, Clone, Debug)]
 #[educe(PartialEq, Eq, Hash)]
 pub struct TemplateBody {
     /// ID of this policy
@@ -1122,7 +1099,7 @@ impl std::fmt::Display for TemplateBody {
 }
 
 /// Template constraint on principal scope variables
-#[derive(Serialize, Deserialize, Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
+#[derive(Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
 pub struct PrincipalConstraint {
     pub(crate) constraint: PrincipalOrResourceConstraint,
 }
@@ -1229,7 +1206,7 @@ impl std::fmt::Display for PrincipalConstraint {
 }
 
 /// Template constraint on resource scope variables
-#[derive(Serialize, Deserialize, Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
+#[derive(Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
 pub struct ResourceConstraint {
     pub(crate) constraint: PrincipalOrResourceConstraint,
 }
@@ -1336,14 +1313,13 @@ impl std::fmt::Display for ResourceConstraint {
 }
 
 /// A reference to an EntityUID that may be a Slot
-#[derive(Educe, Serialize, Deserialize, Clone, Debug, Eq)]
+#[derive(Educe, Clone, Debug, Eq)]
 #[educe(Hash, PartialEq, PartialOrd, Ord)]
 pub enum EntityReference {
     /// Reference to a literal EUID
     EUID(Arc<EntityUID>),
     /// Template Slot
     Slot(
-        #[serde(skip)]
         #[educe(PartialEq(ignore))]
         #[educe(PartialOrd(ignore))]
         #[educe(Hash(ignore))]
@@ -1404,7 +1380,7 @@ impl From<EntityUID> for EntityReference {
 }
 
 /// Subset of AST variables that have the same constraint form
-#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone, Copy)]
+#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
 #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub enum PrincipalOrResource {
     /// The principal of a request
@@ -1435,7 +1411,7 @@ impl TryFrom<Var> for PrincipalOrResource {
 
 /// Represents the constraints for principals and resources.
 /// Can either not constrain, or constrain via `==` or `in` for a single entity literal.
-#[derive(Serialize, Deserialize, Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
+#[derive(Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
 pub enum PrincipalOrResourceConstraint {
     /// Unconstrained
     Any,
@@ -1562,7 +1538,7 @@ impl PrincipalOrResourceConstraint {
 
 /// Constraint for action scope variables.
 /// Action variables can be constrained to be in any variable in a list.
-#[derive(Serialize, Deserialize, Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
+#[derive(Clone, Hash, Eq, PartialEq, PartialOrd, Ord, Debug)]
 pub enum ActionConstraint {
     /// Unconstrained
     Any,
@@ -1875,33 +1851,15 @@ mod test {
     };
 
     #[test]
-    fn literal_and_borrowed() {
+    fn link_templates() {
         for template in all_templates() {
+            template.check_invariant();
             let t = Arc::new(template);
             let env = t
                 .slots()
                 .map(|slot| (slot.id, EntityUID::with_eid("eid")))
                 .collect();
-            let p = Template::link(t, PolicyID::from_string("id"), env).expect("Linking failed");
-
-            let b_literal = BorrowedLiteralPolicy::from(&p);
-            let src = serde_json::to_string(&b_literal).expect("ser error");
-            let literal: LiteralPolicy = serde_json::from_str(&src).expect("de error");
-
-            assert_eq!(b_literal.template_id, &literal.template_id);
-            assert_eq!(b_literal.link_id, literal.link_id.as_ref());
-            assert_eq!(b_literal.values, &literal.values);
-        }
-    }
-
-    #[test]
-    fn template_roundtrip() {
-        for template in all_templates() {
-            template.check_invariant();
-            let json = serde_json::to_string(&template).expect("Serialization Failed");
-            let t2 = serde_json::from_str::<Template>(&json).expect("Deserialization failed");
-            t2.check_invariant();
-            assert_eq!(template, t2);
+            let _ = Template::link(t, PolicyID::from_string("id"), env).expect("Linking failed");
         }
     }
 
diff --git a/cedar-policy-core/src/ast/policy_set.rs b/cedar-policy-core/src/ast/policy_set.rs
index 2b4d7888..fc8e979b 100644
--- a/cedar-policy-core/src/ast/policy_set.rs
+++ b/cedar-policy-core/src/ast/policy_set.rs
@@ -20,15 +20,12 @@ use super::{
 };
 use itertools::Itertools;
 use miette::Diagnostic;
-use serde::{Deserialize, Serialize};
 use std::collections::{hash_map::Entry, HashMap, HashSet};
 use std::{borrow::Borrow, sync::Arc};
 use thiserror::Error;
 
 /// Represents a set of `Policy`s
-#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)]
-#[serde(try_from = "LiteralPolicySet")]
-#[serde(into = "LiteralPolicySet")]
+#[derive(Debug, Default, Clone, PartialEq, Eq)]
 pub struct PolicySet {
     /// `templates` contains all bodies of policies in the `PolicySet`.
     /// A body is either:
@@ -49,8 +46,12 @@ pub struct PolicySet {
     template_to_links_map: HashMap<PolicyID, HashSet<PolicyID>>,
 }
 
-/// A Policy Set that can be serialized, but does not contain as rich information as `PolicySet`
-#[derive(Debug, Serialize, Deserialize)]
+/// A Policy Set that contains less rich information than `PolicySet`.
+///
+/// In particular, this form is easier to convert to/from the Protobuf
+/// representation of a `PolicySet`, because policies are represented as
+/// `LiteralPolicy` instead of `Policy`.
+#[derive(Debug)]
 pub struct LiteralPolicySet {
     /// Like the `templates` field of `PolicySet`
     templates: HashMap<PolicyID, Template>,
diff --git a/cedar-policy-core/src/ast/request.rs b/cedar-policy-core/src/ast/request.rs
index 8b2310fe..b0adada2 100644
--- a/cedar-policy-core/src/ast/request.rs
+++ b/cedar-policy-core/src/ast/request.rs
@@ -33,7 +33,7 @@ use super::{
 };
 
 /// Represents the request tuple <P, A, R, C> (see the Cedar design doc).
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone)]
 pub struct Request {
     /// Principal associated with the request
     pub(crate) principal: EntityUIDEntry,
@@ -64,7 +64,7 @@ pub struct RequestType {
 /// An entry in a request for a Entity UID.
 /// It may either be a concrete EUID
 /// or an unknown in the case of partial evaluation
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone)]
 pub enum EntityUIDEntry {
     /// A concrete EntityUID
     Known {
@@ -275,10 +275,7 @@ impl std::fmt::Display for Request {
 }
 
 /// `Context` field of a `Request`
-#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
-// Serialization is used for differential testing, which requires that `Context`
-// is serialized as a `RestrictedExpr`.
-#[serde(into = "RestrictedExpr")]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub enum Context {
     /// The context is a concrete value.
     Value(Arc<BTreeMap<SmolStr, Value>>),
diff --git a/cedar-policy-core/src/ast/restricted_expr.rs b/cedar-policy-core/src/ast/restricted_expr.rs
index 497489cf..00c3af33 100644
--- a/cedar-policy-core/src/ast/restricted_expr.rs
+++ b/cedar-policy-core/src/ast/restricted_expr.rs
@@ -23,7 +23,6 @@ use crate::extensions::Extensions;
 use crate::parser::err::ParseErrors;
 use crate::parser::{self, Loc};
 use miette::Diagnostic;
-use serde::{Deserialize, Serialize};
 use smol_str::{SmolStr, ToSmolStr};
 use std::hash::{Hash, Hasher};
 use std::ops::Deref;
@@ -51,8 +50,7 @@ use thiserror::Error;
 ///
 /// These restrictions represent the expressions that are allowed to appear as
 /// attribute values in `Slice` and `Context`.
-#[derive(Deserialize, Serialize, Hash, Debug, Clone, PartialEq, Eq)]
-#[serde(transparent)]
+#[derive(Hash, Debug, Clone, PartialEq, Eq)]
 pub struct RestrictedExpr(Expr);
 
 impl RestrictedExpr {
@@ -310,7 +308,7 @@ impl std::str::FromStr for RestrictedExpr {
 ///
 /// We derive `Copy` for this type because it's just a single reference, and
 /// `&T` is `Copy` for all `T`.
-#[derive(Serialize, Hash, Debug, Clone, PartialEq, Eq, Copy)]
+#[derive(Hash, Debug, Clone, PartialEq, Eq, Copy)]
 pub struct BorrowedRestrictedExpr<'a>(&'a Expr);
 
 impl<'a> BorrowedRestrictedExpr<'a> {
diff --git a/cedar-policy-core/src/ast/types.rs b/cedar-policy-core/src/ast/types.rs
index 79612ca7..5fa8c8a9 100644
--- a/cedar-policy-core/src/ast/types.rs
+++ b/cedar-policy-core/src/ast/types.rs
@@ -15,13 +15,12 @@
  */
 
 use crate::ast::EntityType;
-use serde::{Deserialize, Serialize};
 
 use super::Name;
 
 /// This represents the runtime type of a Cedar value.
 /// Nominal types: two entity types are equal if they have the same Name.
-#[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone, Hash, PartialOrd, Ord)]
+#[derive(PartialEq, Eq, Debug, Clone, Hash, PartialOrd, Ord)]
 pub enum Type {
     /// Boolean type
     Bool,
diff --git a/cedar-policy-core/src/ast/value.rs b/cedar-policy-core/src/ast/value.rs
index 39833a2b..82e67d8b 100644
--- a/cedar-policy-core/src/ast/value.rs
+++ b/cedar-policy-core/src/ast/value.rs
@@ -23,16 +23,13 @@ use std::sync::Arc;
 use educe::Educe;
 use itertools::Itertools;
 use miette::Diagnostic;
-use serde::{Deserialize, Serialize};
 use smol_str::SmolStr;
 use thiserror::Error;
 
 /// This describes all the values which could be the dynamic result of evaluating an `Expr`.
 /// Cloning is O(1).
-#[derive(Educe, Debug, Clone, Serialize, Deserialize)]
+#[derive(Educe, Debug, Clone)]
 #[educe(PartialEq, Eq, PartialOrd, Ord)]
-#[serde(into = "Expr")]
-#[serde(try_from = "Expr")]
 pub struct Value {
     /// Underlying actual value
     pub value: ValueKind,
@@ -44,9 +41,7 @@ pub struct Value {
 
 /// This describes all the values which could be the dynamic result of evaluating an `Expr`.
 /// Cloning is O(1).
-#[derive(Debug, Clone, PartialOrd, Ord, Serialize, Deserialize)]
-#[serde(into = "Expr")]
-#[serde(try_from = "Expr")]
+#[derive(Debug, Clone, PartialOrd, Ord)]
 pub enum ValueKind {
     /// anything that is a Literal can also be the dynamic result of evaluating an `Expr`
     Lit(Literal),
diff --git a/cedar-policy-core/src/entities.rs b/cedar-policy-core/src/entities.rs
index 7f64c091..928894a7 100644
--- a/cedar-policy-core/src/entities.rs
+++ b/cedar-policy-core/src/entities.rs
@@ -22,9 +22,6 @@ use crate::transitive_closure::{compute_tc, enforce_tc_and_dag};
 use std::collections::{hash_map, HashMap};
 use std::sync::Arc;
 
-use serde::Serialize;
-use serde_with::serde_as;
-
 /// Module for checking that entities conform with a schema
 pub mod conformance;
 /// Module for error types
@@ -44,29 +41,20 @@ use err::*;
 /// Represents an entity hierarchy, and allows looking up `Entity` objects by
 /// UID.
 //
-/// Note that `Entities` is `Serialize`, but currently this is only used for the
-/// FFI layer in DRT. All others use (and should use) the `from_json_*()` and
-/// `write_to_json()` methods as necessary.
-#[serde_as]
-#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize)]
+/// Note that `Entities` is not `Serialize` itself -- use either the
+/// `from_json_*()` and `write_to_json()` methods here, or the `proto` module in
+/// `cedar-policy`, which is capable of ser/de both Core types like this and
+/// `cedar-policy` types.
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
 pub struct Entities {
-    /// Serde cannot serialize a HashMap to JSON when the key to the map cannot
-    /// be serialized to a JSON string. This is a limitation of the JSON format.
-    /// `serde_as` annotation are used to serialize the data as associative
-    /// lists instead.
-    ///
     /// Important internal invariant: for any `Entities` object that exists,
     /// the `ancestor` relation is transitively closed.
-    #[serde_as(as = "Vec<(_, _)>")]
     entities: HashMap<EntityUID, Arc<Entity>>,
 
     /// The mode flag determines whether this store functions as a partial store or
     /// as a fully concrete store.
     /// Mode::Concrete means that the store is fully concrete, and failed dereferences are an error.
     /// Mode::Partial means the store is partial, and failed dereferences result in a residual.
-    #[serde(default)]
-    #[serde(skip_deserializing)]
-    #[serde(skip_serializing)]
     mode: Mode,
 }
 
diff --git a/cedar-policy-core/src/parser.rs b/cedar-policy-core/src/parser.rs
index 7373fe0b..75b0739f 100644
--- a/cedar-policy-core/src/parser.rs
+++ b/cedar-policy-core/src/parser.rs
@@ -335,7 +335,7 @@ mod tests {
     use super::*;
 
     use crate::ast::test_generators::*;
-    use crate::ast::{Eid, Literal, Template, Value};
+    use crate::ast::{Eid, Literal, Value};
     use crate::evaluator as eval;
     use crate::extensions::Extensions;
     use crate::parser::err::*;
@@ -347,7 +347,7 @@ mod tests {
 
     #[test]
     fn test_template_parsing() {
-        for template in all_templates().map(Template::from) {
+        for template in all_templates() {
             let id = template.id();
             let src = format!("{template}");
             let parsed =
diff --git a/cedar-policy-core/src/parser/loc.rs b/cedar-policy-core/src/parser/loc.rs
index 4c8dc4ad..cd90a553 100644
--- a/cedar-policy-core/src/parser/loc.rs
+++ b/cedar-policy-core/src/parser/loc.rs
@@ -14,12 +14,11 @@
  * limitations under the License.
  */
 
-use serde::{Deserialize, Serialize};
 use std::sync::Arc;
 
 /// Represents a source location: index/range, and a reference to the source
 /// code which that index/range indexes into
-#[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, Serialize, PartialOrd, Ord)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct Loc {
     /// `SourceSpan` indicating a specific source code location or range
     pub span: miette::SourceSpan,
diff --git a/cedar-policy-core/src/parser/node.rs b/cedar-policy-core/src/parser/node.rs
index 5b63457b..364a2541 100644
--- a/cedar-policy-core/src/parser/node.rs
+++ b/cedar-policy-core/src/parser/node.rs
@@ -18,13 +18,12 @@ use std::fmt::{self, Debug, Display};
 
 use educe::Educe;
 use miette::Diagnostic;
-use serde::{Deserialize, Serialize};
 
 use super::err::{ToASTError, ToASTErrorKind};
 use super::loc::Loc;
 
 /// Metadata for our syntax trees
-#[derive(Educe, Debug, Clone, Deserialize, Serialize)]
+#[derive(Educe, Debug, Clone)]
 #[educe(PartialEq, Eq, Hash, Ord, PartialOrd)]
 pub struct Node<T> {
     /// Main data represented
diff --git a/cedar-policy-core/src/parser/text_to_cst.rs b/cedar-policy-core/src/parser/text_to_cst.rs
index b400da2b..b7b9adf1 100644
--- a/cedar-policy-core/src/parser/text_to_cst.rs
+++ b/cedar-policy-core/src/parser/text_to_cst.rs
@@ -1063,15 +1063,7 @@ mod tests {
 
         // In the tolerant AST we store the Policy Error node
         #[cfg(feature = "tolerant-ast")]
-        assert_eq!(
-            policies
-                .clone()
-                .0
-                .into_iter()
-                .filter_map(|p| p.node)
-                .count(),
-            3
-        );
+        assert_eq!(policies.0.into_iter().filter_map(|p| p.node).count(), 3);
 
         // If the AST is not tolerant, unparsable policy should be None
         #[cfg(not(feature = "tolerant-ast"))]
@@ -1475,7 +1467,7 @@ mod tests {
     #[cfg(feature = "tolerant-ast")]
     fn expr_tolerant_success() {
         let src = r#"
-            x == 
+            x ==
         "#;
         let e = assert_parse_succeeds(parse_expr_tolerant, src);
         assert!(matches!(e, Expr::ErrorExpr));
diff --git a/cedar-policy-validator/src/schema.rs b/cedar-policy-validator/src/schema.rs
index 25adf254..f6e3f340 100644
--- a/cedar-policy-validator/src/schema.rs
+++ b/cedar-policy-validator/src/schema.rs
@@ -29,8 +29,7 @@ use cedar_policy_core::{
 };
 use namespace_def::EntityTypeFragment;
 use nonempty::NonEmpty;
-use serde::{Deserialize, Serialize};
-use serde_with::serde_as;
+use serde::Deserialize;
 use smol_str::ToSmolStr;
 use std::collections::{hash_map::Entry, BTreeMap, BTreeSet, HashMap, HashSet};
 use std::str::FromStr;
@@ -151,16 +150,12 @@ impl ValidatorSchemaFragment<ConditionalName, ConditionalName> {
 ///
 /// In this representation, all common types are fully expanded, and all entity
 /// type names are fully disambiguated (fully qualified).
-#[serde_as]
-#[derive(Clone, Debug, Serialize)]
-#[serde(rename_all = "camelCase")]
+#[derive(Clone, Debug)]
 pub struct ValidatorSchema {
     /// Map from entity type names to the [`ValidatorEntityType`] object.
-    #[serde_as(as = "Vec<(_, _)>")]
     entity_types: HashMap<EntityType, ValidatorEntityType>,
 
     /// Map from action id names to the [`ValidatorActionId`] object.
-    #[serde_as(as = "Vec<(_, _)>")]
     action_ids: HashMap<EntityUID, ValidatorActionId>,
 
     /// For easy lookup, this is a map from action name to `Entity` object
@@ -168,7 +163,6 @@ pub struct ValidatorSchema {
     /// in the `ValidatorSchema`, but not efficient to extract -- getting the
     /// `Entity` from the `ValidatorSchema` is O(N) as of this writing, but with
     /// this cache it's O(1).
-    #[serde_as(as = "Vec<(_, _)>")]
     pub(crate) actions: HashMap<EntityUID, Arc<Entity>>,
 }
 
@@ -872,12 +866,12 @@ impl ValidatorSchema {
             }
         }
         action_ids.iter().map(move |(action_id, action)| {
-            Entity::new_with_attr_partial_value_serialized_as_expr(
+            Entity::new_with_attr_partial_value(
                 action_id.clone(),
                 action.attributes.clone(),
                 HashSet::new(),
                 action_ancestors.remove(action_id).unwrap_or_default(),
-                BTreeMap::new(), // actions cannot have entity tags
+                [], // actions cannot have entity tags
             )
         })
     }
diff --git a/cedar-policy-validator/src/schema/action.rs b/cedar-policy-validator/src/schema/action.rs
index e4dd7989..253c89c3 100644
--- a/cedar-policy-validator/src/schema/action.rs
+++ b/cedar-policy-validator/src/schema/action.rs
@@ -17,10 +17,9 @@
 //! This module contains the definition of `ValidatorActionId` and the types it relies on
 
 use cedar_policy_core::{
-    ast::{self, EntityType, EntityUID, PartialValueSerializedAsExpr},
+    ast::{self, EntityType, EntityUID, PartialValue},
     transitive_closure::TCNode,
 };
-use serde::Serialize;
 use smol_str::SmolStr;
 use std::collections::{BTreeMap, HashSet};
 
@@ -35,8 +34,7 @@ use crate::{
 /// Contains information about actions used by the validator.  The contents of
 /// the struct are the same as the schema entity type structure, but the
 /// `member_of` relation is reversed to instead be `descendants`.
-#[derive(Clone, Debug, Serialize)]
-#[serde(rename_all = "camelCase")]
+#[derive(Clone, Debug)]
 pub struct ValidatorActionId {
     /// The name of the action.
     pub(crate) name: EntityUID,
@@ -59,10 +57,7 @@ pub struct ValidatorActionId {
     /// The actual attribute value for this action, used to construct an
     /// `Entity` for this action. Could also be used for more precise
     /// typechecking by partial evaluation.
-    ///
-    /// Attributes are serialized as `RestrictedExpr`s, so that roundtripping
-    /// works seamlessly.
-    pub(crate) attributes: BTreeMap<SmolStr, PartialValueSerializedAsExpr>,
+    pub(crate) attributes: BTreeMap<SmolStr, PartialValue>,
 }
 
 impl ValidatorActionId {
@@ -77,7 +72,7 @@ impl ValidatorActionId {
         descendants: impl IntoIterator<Item = EntityUID>,
         context: Type,
         attribute_types: Attributes,
-        attributes: BTreeMap<SmolStr, PartialValueSerializedAsExpr>,
+        attributes: BTreeMap<SmolStr, PartialValue>,
     ) -> Self {
         Self {
             name,
@@ -150,7 +145,7 @@ impl ValidatorActionId {
     }
 
     /// Attribute values for this action
-    pub fn attributes(&self) -> impl Iterator<Item = (&SmolStr, &PartialValueSerializedAsExpr)> {
+    pub fn attributes(&self) -> impl Iterator<Item = (&SmolStr, &PartialValue)> {
         self.attributes.iter()
     }
 }
@@ -183,8 +178,7 @@ impl TCNode<EntityUID> for ValidatorActionId {
 /// [`InternalName`] and [`Name`] always represents a fully-qualified name, but
 /// as of this writing we always use [`Name`] or [`InternalName`] for the
 /// parameter here when we want to indicate names have been fully qualified.)
-#[derive(Clone, Debug, Serialize)]
-#[serde(rename_all = "camelCase")]
+#[derive(Clone, Debug)]
 pub(crate) struct ValidatorApplySpec<N> {
     /// The principal entity types the action can be applied to.
     principal_apply_spec: HashSet<N>,
diff --git a/cedar-policy-validator/src/schema/namespace_def.rs b/cedar-policy-validator/src/schema/namespace_def.rs
index 4f49b042..ccd5b568 100644
--- a/cedar-policy-validator/src/schema/namespace_def.rs
+++ b/cedar-policy-validator/src/schema/namespace_def.rs
@@ -21,8 +21,8 @@ use std::collections::{hash_map::Entry, BTreeMap, HashMap, HashSet};
 
 use cedar_policy_core::{
     ast::{
-        EntityAttrEvaluationError, EntityType, EntityUID, InternalName, Name,
-        PartialValueSerializedAsExpr, UnreservedId,
+        EntityAttrEvaluationError, EntityType, EntityUID, InternalName, Name, PartialValue,
+        UnreservedId,
     },
     entities::{json::err::JsonDeserializationErrorContext, CedarValueJson},
     evaluator::RestrictedEvaluator,
@@ -696,7 +696,7 @@ pub struct ActionFragment<N, A> {
     /// The values for the attributes defined for this actions entity, stored
     /// separately so that we can later extract these values to construct the
     /// actual `Entity` objects defined by the schema.
-    pub(super) attributes: BTreeMap<SmolStr, PartialValueSerializedAsExpr>,
+    pub(super) attributes: BTreeMap<SmolStr, PartialValue>,
 }
 
 impl ActionFragment<ConditionalName, ConditionalName> {
@@ -777,9 +777,9 @@ impl ActionFragment<ConditionalName, ConditionalName> {
         m: HashMap<SmolStr, CedarValueJson>,
         action_id: &EntityUID,
         extensions: &Extensions<'_>,
-    ) -> crate::err::Result<(Attributes, BTreeMap<SmolStr, PartialValueSerializedAsExpr>)> {
+    ) -> crate::err::Result<(Attributes, BTreeMap<SmolStr, PartialValue>)> {
         let mut attr_types: HashMap<SmolStr, Type> = HashMap::with_capacity(m.len());
-        let mut attr_values: BTreeMap<SmolStr, PartialValueSerializedAsExpr> = BTreeMap::new();
+        let mut attr_values: BTreeMap<SmolStr, PartialValue> = BTreeMap::new();
         let evaluator = RestrictedEvaluator::new(extensions);
 
         for (k, v) in m {
@@ -810,7 +810,7 @@ impl ActionFragment<ConditionalName, ConditionalName> {
                         err,
                     })
                 })?;
-            attr_values.insert(k.clone(), pv.into());
+            attr_values.insert(k, pv);
         }
         Ok((
             Attributes::with_required_attributes(attr_types),
diff --git a/cedar-policy/src/proto/ast.rs b/cedar-policy/src/proto/ast.rs
index 4eed04a2..ca2fa5b5 100644
--- a/cedar-policy/src/proto/ast.rs
+++ b/cedar-policy/src/proto/ast.rs
@@ -20,11 +20,7 @@ use super::models;
 use cedar_policy_core::{
     ast, evaluator::RestrictedEvaluator, extensions::Extensions, FromNormalizedStr,
 };
-use smol_str::SmolStr;
-use std::{
-    collections::{BTreeMap, HashSet},
-    sync::Arc,
-};
+use std::{collections::HashSet, sync::Arc};
 
 // PANIC SAFETY: experimental feature
 #[allow(clippy::fallible_impl_from)]
@@ -131,37 +127,29 @@ impl From<&models::Entity> for ast::Entity {
     fn from(v: &models::Entity) -> Self {
         let eval = RestrictedEvaluator::new(Extensions::none());
 
-        let attrs: BTreeMap<SmolStr, ast::PartialValueSerializedAsExpr> = v
-            .attrs
-            .iter()
-            .map(|(key, value)| {
-                let pval = eval
-                    .partial_interpret(
-                        ast::BorrowedRestrictedExpr::new(&ast::Expr::from(value)).unwrap(),
-                    )
-                    .expect("interpret on RestrictedExpr");
-                (key.into(), pval.into())
-            })
-            .collect();
+        let attrs = v.attrs.iter().map(|(key, value)| {
+            let pval = eval
+                .partial_interpret(
+                    ast::BorrowedRestrictedExpr::new(&ast::Expr::from(value)).unwrap(),
+                )
+                .expect("interpret on RestrictedExpr");
+            (key.into(), pval)
+        });
 
         let ancestors: HashSet<ast::EntityUID> =
             v.ancestors.iter().map(ast::EntityUID::from).collect();
 
-        let tags: BTreeMap<SmolStr, ast::PartialValueSerializedAsExpr> = v
-            .tags
-            .iter()
-            .map(|(key, value)| {
-                let pval = eval
-                    .partial_interpret(
-                        ast::BorrowedRestrictedExpr::new(&ast::Expr::from(value))
-                            .expect("RestrictedExpr"),
-                    )
-                    .expect("interpret on RestrictedExpr");
-                (key.into(), pval.into())
-            })
-            .collect();
+        let tags = v.tags.iter().map(|(key, value)| {
+            let pval = eval
+                .partial_interpret(
+                    ast::BorrowedRestrictedExpr::new(&ast::Expr::from(value))
+                        .expect("RestrictedExpr"),
+                )
+                .expect("interpret on RestrictedExpr");
+            (key.into(), pval)
+        });
 
-        Self::new_with_attr_partial_value_serialized_as_expr(
+        Self::new_with_attr_partial_value(
             ast::EntityUID::from(v.uid.as_ref().expect("uid field should exist")),
             attrs,
             ancestors,
@@ -669,8 +657,6 @@ impl From<&ast::Context> for models::Expr {
 
 #[cfg(test)]
 mod test {
-    use std::collections::HashMap;
-
     use super::*;
 
     #[test]
@@ -722,15 +708,13 @@ mod test {
             ast::EntityUID::from(&models::EntityUid::from(&euid3))
         );
 
-        let attrs = (1..=7)
-            .map(|id| (format!("{id}").into(), ast::RestrictedExpr::val(true)))
-            .collect::<HashMap<SmolStr, _>>();
+        let attrs = (1..=7).map(|id| (format!("{id}").into(), ast::RestrictedExpr::val(true)));
         let entity = ast::Entity::new(
             r#"Foo::"bar""#.parse().unwrap(),
             attrs,
             HashSet::new(),
             HashSet::new(),
-            BTreeMap::new(),
+            [],
             Extensions::none(),
         )
         .unwrap();


Test patch:
diff --git a/cedar-testing/src/cedar_test_impl.rs b/cedar-testing/src/cedar_test_impl.rs
index dd1510fc..cde4d116 100644
--- a/cedar-testing/src/cedar_test_impl.rs
+++ b/cedar-testing/src/cedar_test_impl.rs
@@ -29,7 +29,7 @@ use cedar_policy_core::extensions::Extensions;
 use cedar_policy_validator::{ValidationMode, Validator, ValidatorSchema};
 use core::panic;
 use miette::miette;
-use serde::{Deserialize, Serialize};
+use serde::Deserialize;
 use std::collections::HashMap;
 use std::collections::HashSet;
 use std::time::{Duration, Instant};
@@ -133,7 +133,7 @@ impl TestValidationResult {
     }
 }
 
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone)]
 pub enum ExprOrValue {
     Expr(Expr),
     Value(Expr),