From 25f7d950cadcfecddaecc2f3ba5af99a0adde0bf Mon Sep 17 00:00:00 2001
From: Craig Disselkoen <cdiss@amazon.com>
Date: Tue, 25 Feb 2025 12:33:55 -0800
Subject: [PATCH] renaming, inlining, and commenting in .proto files (#1488)

Signed-off-by: Craig Disselkoen <cdiss@amazon.com>
---
 cedar-policy-core/src/ast/policy.rs          |  11 +-
 cedar-policy/CHANGELOG.md                    |   3 +
 cedar-policy/protobuf_schema/core.proto      | 140 +++++------
 cedar-policy/protobuf_schema/validator.proto |  78 +++---
 cedar-policy/src/proto/api.rs                |  28 +--
 cedar-policy/src/proto/ast.rs                | 101 ++------
 cedar-policy/src/proto/policy.rs             | 191 +++++++--------
 cedar-policy/src/proto/validator.rs          | 235 ++++++-------------
 8 files changed, 318 insertions(+), 469 deletions(-)

diff --git a/cedar-policy-core/src/ast/policy.rs b/cedar-policy-core/src/ast/policy.rs
index 6e2b83ab0..6d1791ce1 100644
--- a/cedar-policy-core/src/ast/policy.rs
+++ b/cedar-policy-core/src/ast/policy.rs
@@ -355,9 +355,9 @@ fn describe_arity_error(
 }
 
 /// A Policy that contains:
-///   a pointer to its template
-///   an link ID (unless it's an static policy)
-///   the bound values for slots in the template
+///   - a pointer to its template
+///   - a link ID (unless it's a static policy)
+///   - the bound values for slots in the template
 ///
 /// Policies are not serializable (due to the pointer), and can be serialized
 /// by converting to/from LiteralPolicy
@@ -366,10 +366,12 @@ pub struct Policy {
     /// Reference to the template
     template: Arc<Template>,
     /// Id of this link
+    ///
     /// None in the case that this is an instance of a Static Policy
     link: Option<PolicyID>,
     // INVARIANT (values total map)
     // All of the slots in `template` MUST be bound by `values`
+    //
     /// values the slots are bound to.
     /// The constructor `new` is only visible in this module,
     /// so it is the responsibility of callers to maintain
@@ -561,7 +563,8 @@ impl std::fmt::Display for Policy {
 /// Map from Slot Ids to Entity UIDs which fill the slots
 pub type SlotEnv = HashMap<SlotId, EntityUID>;
 
-/// Represents either an static policy or a template linked policy
+/// 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)]
diff --git a/cedar-policy/CHANGELOG.md b/cedar-policy/CHANGELOG.md
index 2d578ea1b..a1abf0202 100644
--- a/cedar-policy/CHANGELOG.md
+++ b/cedar-policy/CHANGELOG.md
@@ -16,6 +16,9 @@ Cedar Language Version: TBD
 ### Changed
 
 - Changed `Entities::add_entities` and `Entities::from_entities` to ignore structurally equal entities with the same Entity UID.
+- For `protobufs` experimental feature, a number of changes to the interface and
+  the Protobuf format definitions, as we continue to iterate towards making this
+  feature stable.
 
 ### Added
 
diff --git a/cedar-policy/protobuf_schema/core.proto b/cedar-policy/protobuf_schema/core.proto
index 863111f8c..757bfcf33 100644
--- a/cedar-policy/protobuf_schema/core.proto
+++ b/cedar-policy/protobuf_schema/core.proto
@@ -18,16 +18,26 @@ syntax = "proto3";
 package cedar_policy_core;
 
 message Request {
-    EntityUidEntry principal = 1;
-    EntityUidEntry action = 2;
-    EntityUidEntry resource = 3;
-    Context context = 4;
+    EntityUid principal = 1;
+    EntityUid action = 2;
+    EntityUid resource = 3;
+    Expr context = 4;
 }
 
-message LiteralPolicySet {
-    // Key is PolicyID as a string
+// the protobuf PolicySet message describes a complete policy set, including
+// templates, static policies, and/or template-linked policies.
+message PolicySet {
+    // Key is PolicyID as a string.
+    // Value is a `TemplateBody`.
+    // Both templates and static policies are included in this map, with static
+    // policies represented as templates with zero slots.
     map<string, TemplateBody> templates = 1;
-    map<string, LiteralPolicy> links = 2;
+    // Key is PolicyID as a string.
+    // Value is a `Policy`.
+    // All static policies and template-linked policies are included in this map.
+    // Static policies must have exactly one entry in this map, and the PolicyID
+    // of the static policy must be the same in this map and the above map.
+    map<string, Policy> links = 2;
 }
 
 enum Mode {
@@ -40,52 +50,35 @@ message Entities {
     Mode mode = 2;
 }
 
-message Context {
-    Expr context = 1;
-}
-
-// BEGIN REQUEST MESSAGES
-
-message EntityUidEntry {
-    EntityUid euid = 1;
-}
-
 message EntityUid {
-    EntityType ty = 1;
+    Name ty = 1;
     string eid = 2;
 }
 
-message EntityType {
-    Name name = 1;
-}
-
-// alias Id = string
 message Name {
     string id = 1;
     repeated string path = 2;
 }
 
-
-// END REQUEST MESSAGES
-
-
-// BEGIN POLICYSET MESSAGES
-
-message LiteralPolicy {
+// the protobuf Policy message describes either a static or a template-linked policy.
+message Policy {
+    // ID of the template associated with this policy.
+    // For static policies, this is the ID of a zero-slot template.
     string template_id = 1;
-    string link_id = 2;
-    bool link_id_specified = 3;
-    // map<SlotId, EntityUid> is not allowed since keys in map
-    // fields cannot be enum types
-    // map<SlotId, EntityUid> values = 4;
+    // ID of this policy itself.
+    // For static policies, this is omitted/ignored; the ID of the policy is the
+    // `template_id`.
+    optional string link_id = 2;
+    // Whether this policy is a static (false) or template-linked (true) policy
+    bool is_template_link = 3;
+    // Value of the `?principal` slot.
+    // Omitted/ignored for templates without the `?principal` slot.
     EntityUid principal_euid = 4;
+    // Value of the `?resource` slot.
+    // Omitted/ignored for templates without the `?resource` slot.
     EntityUid resource_euid = 5;
 }
 
-message Annotation {
-    string val = 1;
-}
-
 enum Effect {
     Forbid = 0;
     Permit = 1;
@@ -93,48 +86,42 @@ enum Effect {
 
 message TemplateBody {
     string id = 1;
-    // alias AnyId = string
-    // alias Annotations = map<AnyId, Annotation>
-    map<string, Annotation> annotations = 3;
+    map<string, string> annotations = 3;
     Effect effect = 4;
-    PrincipalConstraint principal_constraint = 5;
+    PrincipalOrResourceConstraint principal_constraint = 5;
     ActionConstraint action_constraint = 6;
-    ResourceConstraint resource_constraint = 7;
+    PrincipalOrResourceConstraint resource_constraint = 7;
     Expr non_scope_constraints = 8;
 }
 
-message PrincipalConstraint {
-    PrincipalOrResourceConstraint constraint = 1;
-}
-
-message ResourceConstraint {
-    PrincipalOrResourceConstraint constraint = 1;
-}
-
+// an EntityReference may either be an EntityUid or a Slot.
 message EntityReference {
     oneof data {
-        Ty ty = 1;
+        Slot slot = 1;
         EntityUid euid = 2;
     }
 
-    // Zero-Arity constructors
-    enum Ty {
-        Slot = 0;
+    // if it's a Slot, we know from context which Slot it is,
+    // so we don't need a `SlotId` and can just use this one-armed enum
+    enum Slot {
+        // the one option for the enum
+        unit = 0;
     }
 }
 
 message PrincipalOrResourceConstraint {
     oneof data {
-        Ty ty = 1;
+        Any any = 1;
         InMessage in = 2;
         EqMessage eq = 3;
         IsMessage is = 4;
         IsInMessage isIn = 5;
     }
 
-    // Zero-arity constructors
-    enum Ty {
-        Any = 0;
+    // Zero-arity constructors represented as enums with only one member
+    enum Any {
+        // the one option for the enum
+        unit = 0;
     }
 
     message InMessage {
@@ -144,11 +131,11 @@ message PrincipalOrResourceConstraint {
         EntityReference er = 1;
     }
     message IsMessage {
-        EntityType et = 1;
+        Name entity_type = 1;
     }
     message IsInMessage {
         EntityReference er = 1;
-        EntityType et = 2;
+        Name entity_type = 2;
     }
 }
 
@@ -159,14 +146,17 @@ enum SlotId {
 
 message ActionConstraint {
     oneof data {
-        Ty ty = 1;
+        Any any = 1;
         InMessage in = 2;
         EqMessage eq = 3;
     }
 
-    enum Ty {
-        Any = 0;
+    // Zero-arity constructors represented as enums with only one member
+    enum Any {
+        // the one option for the enum
+        unit = 0;
     }
+
     message InMessage {
         repeated EntityUid euids = 1;
     }
@@ -210,7 +200,7 @@ message Expr {
         Principal = 0;
         Action = 1;
         Resource = 2;
-        CONTEXT = 3;
+        Context = 3;
     }
 
     message If {
@@ -282,20 +272,21 @@ message Expr {
 
         message PatternElem {
             oneof data {
-                Ty ty = 1;
+                Wildcard wildcard = 1;
                 string c = 2;
             }
 
-            // Zero-arity constructors
-            enum Ty {
-                Wildcard = 0;
+            // Zero-arity constructors represented as enums with only one member
+            enum Wildcard {
+                // the one option for the enum
+                unit = 0;
             }
         }
     }
 
     message Is {
         Expr expr = 1;
-        EntityType entity_type = 2;
+        Name entity_type = 2;
     }
 
     message Set {
@@ -307,16 +298,9 @@ message Expr {
     }
 }
 
-// END POLICYSET MESSAGES
-
-
-// ENTER ENTITIES MESSAGES
-
 message Entity {
     EntityUid uid = 1;
     map<string, Expr> attrs = 2;
     repeated EntityUid ancestors = 3;
     map<string, Expr> tags = 4;
 }
-
-// END ENTITIES MESSAGES
diff --git a/cedar-policy/protobuf_schema/validator.proto b/cedar-policy/protobuf_schema/validator.proto
index c6317df56..209b9a28e 100644
--- a/cedar-policy/protobuf_schema/validator.proto
+++ b/cedar-policy/protobuf_schema/validator.proto
@@ -19,44 +19,47 @@ package cedar_policy_validator;
 
 import "core.proto";
 
-message ValidatorSchema {
-    repeated EntityTypeWithTypesMap entity_types = 1;
-    repeated EntityUidWithActionIdsMap action_ids = 2;
+// the protobuf Schema message describes a complete schema.
+message Schema {
+    // TODO: this need not be a map at all, since `EntityDecl` contains the `name` as well.
+    // It can be just `repeated EntityDecl`.
+    repeated EntityTypeToEntityDeclMap entity_decls = 1;
+    // TODO: this need not be a map at all, since `ActionDecl` contains the `name` as well.
+    // It can be just `repeated ActionDecl`.
+    repeated EntityUidToActionDeclMap action_decls = 2;
 }
 
-// Workaround since messages can't be dictionary keys
-message EntityTypeWithTypesMap {
-    cedar_policy_core.EntityType key = 1;
-    ValidatorEntityType value = 2;
+// This `message` with `key` and `value`, rather than a `map`, since messages can't be dictionary keys
+message EntityTypeToEntityDeclMap {
+    cedar_policy_core.Name key = 1;
+    EntityDecl value = 2;
 }
 
-message EntityUidWithActionIdsMap {
+// This `message` with `key` and `value`, rather than a `map`, since messages can't be dictionary keys
+message EntityUidToActionDeclMap {
     cedar_policy_core.EntityUid key = 1;
-    ValidatorActionId value = 2;
+    ActionDecl value = 2;
 }
 
-message ValidatorEntityType {
-    cedar_policy_core.EntityType name = 1;
-    repeated cedar_policy_core.EntityType descendants = 2;
-    Attributes attributes = 3;
+// the protobuf EntityDecl message contains all of the schema's
+// information about a single entity type.
+message EntityDecl {
+    cedar_policy_core.Name name = 1;
+    repeated cedar_policy_core.Name descendants = 2;
+    map<string, AttributeType> attributes = 3;
     OpenTag open_attributes = 4;
-    Tag tags = 5;
+    optional Type tags = 5;
     repeated string enum_choices = 6;
 }
 
-message ValidatorActionId {
+// the protobuf ActionDecl message contains all of the schema's
+// information about a single action.
+message ActionDecl {
     cedar_policy_core.EntityUid name = 1;
-    ValidatorApplySpec applies_to = 2;
     repeated cedar_policy_core.EntityUid descendants = 3;
     Type context = 4;
-    Attributes attribute_types = 5;
-    // Deserialize Expr as Value
-    map<string, cedar_policy_core.Expr> attributes = 6;
-}
-
-message ValidatorApplySpec {
-    repeated cedar_policy_core.EntityType principal_apply_spec = 1;
-    repeated cedar_policy_core.EntityType resource_apply_spec = 2;
+    repeated cedar_policy_core.Name principal_types = 7;
+    repeated cedar_policy_core.Name resource_types = 8;
 }
 
 message Type {
@@ -80,25 +83,24 @@ message Type {
 
 message EntityRecordKind {
     oneof data {
-        Ty ty = 1;
+        AnyEntity any_entity = 1;
         Record record = 2;
-        Entity entity = 3;
+        cedar_policy_core.Name entity = 3;
         ActionEntity actionEntity = 4;
     }
 
-    enum Ty {
-        AnyEntity = 0; // AnyEntity
+    // Zero-arity constructors represented as enums with only one member
+    enum AnyEntity {
+        // the one option for the enum
+        unit = 0;
     }
     message Record {
-        Attributes attrs = 1;
+        map<string, AttributeType> attrs = 1;
         OpenTag open_attributes = 2;
     }
-    message Entity {
-        cedar_policy_core.EntityType e = 1;
-    }
     message ActionEntity {
-        cedar_policy_core.EntityType name = 1;
-        Attributes attrs = 2;
+        cedar_policy_core.Name name = 1;
+        map<string, AttributeType> attrs = 2;
     }
 }
 
@@ -107,19 +109,11 @@ enum OpenTag {
     ClosedAttributes = 1;
 }
 
-message Attributes {
-    map<string, AttributeType> attrs = 1;
-}
-
 message AttributeType {
     Type attr_type = 1;
     bool is_required = 2;
 }
 
-message Tag {
-    Type optional_type = 1;
-}
-
 enum ValidationMode {
     Strict = 0;
     Permissive = 1;
diff --git a/cedar-policy/src/proto/api.rs b/cedar-policy/src/proto/api.rs
index a2f7c035e..dae62b2f6 100644
--- a/cedar-policy/src/proto/api.rs
+++ b/cedar-policy/src/proto/api.rs
@@ -40,7 +40,7 @@ macro_rules! standard_conversions {
 
 standard_conversions!(api::Entity, api::Entity, models::Entity);
 standard_conversions!(api::Entities, api::Entities, models::Entities);
-standard_conversions!(api::Schema, api::Schema, models::ValidatorSchema);
+standard_conversions!(api::Schema, api::Schema, models::Schema);
 standard_conversions!(api::EntityTypeName, api::EntityTypeName, models::Name);
 standard_conversions!(api::EntityNamespace, api::EntityNamespace, models::Name);
 standard_conversions!(api::Expression, api::Expression, models::Expr);
@@ -60,29 +60,29 @@ impl From<&models::TemplateBody> for api::Template {
     }
 }
 
-impl From<&api::Policy> for models::LiteralPolicy {
+impl From<&api::Policy> for models::Policy {
     fn from(v: &api::Policy) -> Self {
         Self::from(&v.ast)
     }
 }
 
-impl TryFrom<&models::LiteralPolicy> for api::Policy {
+impl TryFrom<&models::Policy> for api::Policy {
     type Error = cedar_policy_core::ast::ReificationError;
-    fn try_from(v: &models::LiteralPolicy) -> Result<Self, Self::Error> {
+    fn try_from(v: &models::Policy) -> Result<Self, Self::Error> {
         let p = cedar_policy_core::ast::Policy::try_from(v)?;
         Ok(Self::from_ast(p))
     }
 }
 
-impl From<&api::PolicySet> for models::LiteralPolicySet {
+impl From<&api::PolicySet> for models::PolicySet {
     fn from(v: &api::PolicySet) -> Self {
         Self::from(&v.ast)
     }
 }
 
-impl TryFrom<&models::LiteralPolicySet> for api::PolicySet {
+impl TryFrom<&models::PolicySet> for api::PolicySet {
     type Error = api::PolicySetError;
-    fn try_from(v: &models::LiteralPolicySet) -> Result<Self, Self::Error> {
+    fn try_from(v: &models::PolicySet) -> Result<Self, Self::Error> {
         // PANIC SAFETY: experimental feature
         #[allow(clippy::expect_used)]
         Self::from_ast(
@@ -111,7 +111,7 @@ macro_rules! standard_protobuf_impl {
 
 standard_protobuf_impl!(api::Entity, models::Entity);
 standard_protobuf_impl!(api::Entities, models::Entities);
-standard_protobuf_impl!(api::Schema, models::ValidatorSchema);
+standard_protobuf_impl!(api::Schema, models::Schema);
 standard_protobuf_impl!(api::EntityTypeName, models::Name);
 standard_protobuf_impl!(api::EntityNamespace, models::Name);
 standard_protobuf_impl!(api::Template, models::TemplateBody);
@@ -122,26 +122,24 @@ standard_protobuf_impl!(api::Request, models::Request);
 
 impl traits::Protobuf for api::PolicySet {
     fn encode(&self) -> Vec<u8> {
-        traits::encode_to_vec::<models::LiteralPolicySet>(self)
+        traits::encode_to_vec::<models::PolicySet>(self)
     }
     fn decode(buf: impl prost::bytes::Buf) -> Result<Self, prost::DecodeError> {
         // PANIC SAFETY: experimental feature
         #[allow(clippy::expect_used)]
-        Ok(
-            traits::try_decode::<models::LiteralPolicySet, _, Self>(buf)?
-                .expect("protobuf-encoded policy set should be a valid policy set"),
-        )
+        Ok(traits::try_decode::<models::PolicySet, _, Self>(buf)?
+            .expect("protobuf-encoded policy set should be a valid policy set"))
     }
 }
 
 impl traits::Protobuf for api::Policy {
     fn encode(&self) -> Vec<u8> {
-        traits::encode_to_vec::<models::LiteralPolicy>(self)
+        traits::encode_to_vec::<models::Policy>(self)
     }
     fn decode(buf: impl prost::bytes::Buf) -> Result<Self, prost::DecodeError> {
         // PANIC SAFETY: experimental feature
         #[allow(clippy::expect_used)]
-        Ok(traits::try_decode::<models::LiteralPolicy, _, Self>(buf)?
+        Ok(traits::try_decode::<models::Policy, _, Self>(buf)?
             .expect("protobuf-encoded policy should be a valid policy"))
     }
 }
diff --git a/cedar-policy/src/proto/ast.rs b/cedar-policy/src/proto/ast.rs
index 3a2905fff..292288433 100644
--- a/cedar-policy/src/proto/ast.rs
+++ b/cedar-policy/src/proto/ast.rs
@@ -26,23 +26,6 @@ use std::{
     sync::Arc,
 };
 
-impl From<&models::Annotation> for ast::Annotation {
-    fn from(v: &models::Annotation) -> Self {
-        Self {
-            val: v.val.clone().into(),
-            loc: None,
-        }
-    }
-}
-
-impl From<&ast::Annotation> for models::Annotation {
-    fn from(v: &ast::Annotation) -> Self {
-        Self {
-            val: v.val.to_string(),
-        }
-    }
-}
-
 // PANIC SAFETY: experimental feature
 #[allow(clippy::fallible_impl_from)]
 impl From<&models::Name> for ast::InternalName {
@@ -98,28 +81,6 @@ impl From<&ast::EntityType> for models::Name {
     }
 }
 
-impl From<&models::EntityType> for ast::Name {
-    // PANIC SAFETY: experimental feature
-    #[allow(clippy::expect_used, clippy::fallible_impl_from)]
-    fn from(v: &models::EntityType) -> Self {
-        Self::from(v.name.as_ref().expect("name field should exist"))
-    }
-}
-
-impl From<&models::EntityType> for ast::EntityType {
-    fn from(v: &models::EntityType) -> Self {
-        Self::from(ast::Name::from(v))
-    }
-}
-
-impl From<&ast::EntityType> for models::EntityType {
-    fn from(v: &ast::EntityType) -> Self {
-        Self {
-            name: Some(models::Name::from(v.name())),
-        }
-    }
-}
-
 impl From<&models::EntityUid> for ast::EntityUID {
     // PANIC SAFETY: experimental feature
     #[allow(clippy::expect_used)]
@@ -135,24 +96,21 @@ impl From<&models::EntityUid> for ast::EntityUID {
 impl From<&ast::EntityUID> for models::EntityUid {
     fn from(v: &ast::EntityUID) -> Self {
         Self {
-            ty: Some(models::EntityType::from(v.entity_type())),
+            ty: Some(models::Name::from(v.entity_type())),
             eid: <ast::Eid as AsRef<str>>::as_ref(v.eid()).into(),
         }
     }
 }
 
-impl From<&models::EntityUidEntry> for ast::EntityUIDEntry {
-    fn from(v: &models::EntityUidEntry) -> Self {
+impl From<&models::EntityUid> for ast::EntityUIDEntry {
+    fn from(v: &models::EntityUid) -> Self {
         // PANIC SAFETY: experimental feature
         #[allow(clippy::expect_used)]
-        ast::EntityUIDEntry::known(
-            ast::EntityUID::from(v.euid.as_ref().expect("euid field should exist")),
-            None,
-        )
+        ast::EntityUIDEntry::known(ast::EntityUID::from(v), None)
     }
 }
 
-impl From<&ast::EntityUIDEntry> for models::EntityUidEntry {
+impl From<&ast::EntityUIDEntry> for models::EntityUid {
     // PANIC SAFETY: experimental feature
     #[allow(clippy::unimplemented)]
     fn from(v: &ast::EntityUIDEntry) -> Self {
@@ -162,9 +120,7 @@ impl From<&ast::EntityUIDEntry> for models::EntityUidEntry {
                     "Unknown EntityUID is not currently supported by the Protobuf interface"
                 );
             }
-            ast::EntityUIDEntry::Known { euid, .. } => Self {
-                euid: Some(models::EntityUid::from(euid.as_ref())),
-            },
+            ast::EntityUIDEntry::Known { euid, .. } => models::EntityUid::from(euid.as_ref()),
         }
     }
 }
@@ -466,7 +422,7 @@ impl From<&ast::Expr> for models::Expr {
             ast::ExprKind::Is { expr, entity_type } => {
                 models::expr::expr_kind::Data::Is(Box::new(models::expr::Is {
                     expr: Some(Box::new(models::Expr::from(expr.as_ref()))),
-                    entity_type: Some(models::EntityType::from(entity_type)),
+                    entity_type: Some(models::Name::from(entity_type)),
                 }))
             }
             ast::ExprKind::Set(args) => {
@@ -645,11 +601,11 @@ impl From<&models::expr::like::PatternElem> for ast::PatternElem {
                 ast::PatternElem::Char(c.chars().next().expect("c is non-empty"))
             }
 
-            models::expr::like::pattern_elem::Data::Ty(ty) => {
-                match models::expr::like::pattern_elem::Ty::try_from(ty.to_owned())
+            models::expr::like::pattern_elem::Data::Wildcard(unit) => {
+                match models::expr::like::pattern_elem::Wildcard::try_from(*unit)
                     .expect("decode should succeed")
                 {
-                    models::expr::like::pattern_elem::Ty::Wildcard => ast::PatternElem::Wildcard,
+                    models::expr::like::pattern_elem::Wildcard::Unit => ast::PatternElem::Wildcard,
                 }
             }
         }
@@ -663,8 +619,8 @@ impl From<&ast::PatternElem> for models::expr::like::PatternElem {
                 data: Some(models::expr::like::pattern_elem::Data::C(c.to_string())),
             },
             ast::PatternElem::Wildcard => Self {
-                data: Some(models::expr::like::pattern_elem::Data::Ty(
-                    models::expr::like::pattern_elem::Ty::Wildcard.into(),
+                data: Some(models::expr::like::pattern_elem::Data::Wildcard(
+                    models::expr::like::pattern_elem::Wildcard::Unit.into(),
                 )),
             },
         }
@@ -687,36 +643,30 @@ impl From<&models::Request> for ast::Request {
 impl From<&ast::Request> for models::Request {
     fn from(v: &ast::Request) -> Self {
         Self {
-            principal: Some(models::EntityUidEntry::from(v.principal())),
-            action: Some(models::EntityUidEntry::from(v.action())),
-            resource: Some(models::EntityUidEntry::from(v.resource())),
-            context: v.context().map(models::Context::from),
+            principal: Some(models::EntityUid::from(v.principal())),
+            action: Some(models::EntityUid::from(v.action())),
+            resource: Some(models::EntityUid::from(v.resource())),
+            context: v.context().map(models::Expr::from),
         }
     }
 }
 
-impl From<&models::Context> for ast::Context {
-    fn from(v: &models::Context) -> Self {
+impl From<&models::Expr> for ast::Context {
+    fn from(v: &models::Expr) -> Self {
         // PANIC SAFETY: experimental feature
         #[allow(clippy::expect_used)]
         ast::Context::from_expr(
-            ast::BorrowedRestrictedExpr::new(&ast::Expr::from(
-                v.context.as_ref().expect("context.as_ref()"),
-            ))
-            .expect("Expr::from"),
+            ast::BorrowedRestrictedExpr::new(&ast::Expr::from(v))
+                .expect("context should be valid restricted expr"),
             Extensions::none(),
         )
         .expect("Context::from_expr")
     }
 }
 
-impl From<&ast::Context> for models::Context {
+impl From<&ast::Context> for models::Expr {
     fn from(v: &ast::Context) -> Self {
-        Self {
-            context: Some(models::Expr::from(&ast::Expr::from(
-                ast::PartialValue::from(v.to_owned()),
-            ))),
-        }
+        models::Expr::from(&ast::Expr::from(ast::PartialValue::from(v.to_owned())))
     }
 }
 
@@ -732,7 +682,7 @@ mod test {
         let ety_specified = ast::EntityType::from(name);
         assert_eq!(
             ety_specified,
-            ast::EntityType::from(&models::EntityType::from(&ety_specified))
+            ast::EntityType::from(&models::Name::from(&ety_specified))
         );
 
         let euid1 = ast::EntityUID::with_eid_and_type("A", "foo").unwrap();
@@ -898,10 +848,7 @@ mod test {
             Some(context.clone()),
         );
         let request_rt = ast::Request::from(&models::Request::from(&request));
-        assert_eq!(
-            context,
-            ast::Context::from(&models::Context::from(&context))
-        );
+        assert_eq!(context, ast::Context::from(&models::Expr::from(&context)));
         assert_eq!(request.principal().uid(), request_rt.principal().uid());
         assert_eq!(request.action().uid(), request_rt.action().uid());
         assert_eq!(request.resource().uid(), request_rt.resource().uid());
diff --git a/cedar-policy/src/proto/policy.rs b/cedar-policy/src/proto/policy.rs
index 2b839c1a1..664c5dd43 100644
--- a/cedar-policy/src/proto/policy.rs
+++ b/cedar-policy/src/proto/policy.rs
@@ -20,10 +20,10 @@ use super::models;
 use cedar_policy_core::{ast, FromNormalizedStr};
 use std::collections::HashMap;
 
-impl From<&models::LiteralPolicy> for ast::LiteralPolicy {
+impl From<&models::Policy> for ast::LiteralPolicy {
     // PANIC SAFETY: experimental feature
     #[allow(clippy::expect_used)]
-    fn from(v: &models::LiteralPolicy) -> Self {
+    fn from(v: &models::Policy) -> Self {
         let mut values: ast::SlotEnv = HashMap::new();
         if v.principal_euid.is_some() {
             values.insert(
@@ -48,10 +48,10 @@ impl From<&models::LiteralPolicy> for ast::LiteralPolicy {
 
         let template_id = ast::PolicyID::from_string(v.template_id.clone());
 
-        if v.link_id_specified {
+        if v.is_template_link {
             Self::template_linked_policy(
                 template_id,
-                ast::PolicyID::from_string(v.link_id.clone()),
+                ast::PolicyID::from_string(v.link_id.as_ref().expect("link_id field should exist")),
                 values,
             )
         } else {
@@ -60,24 +60,24 @@ impl From<&models::LiteralPolicy> for ast::LiteralPolicy {
     }
 }
 
-impl TryFrom<&models::LiteralPolicy> for ast::Policy {
+impl TryFrom<&models::Policy> for ast::Policy {
     type Error = ast::ReificationError;
-    fn try_from(policy: &models::LiteralPolicy) -> Result<Self, Self::Error> {
+    fn try_from(policy: &models::Policy) -> Result<Self, Self::Error> {
         // TODO: do we need to provide a nonempty `templates` argument to `.reify()`
         ast::LiteralPolicy::from(policy).reify(&HashMap::new())
     }
 }
 
-impl From<&ast::LiteralPolicy> for models::LiteralPolicy {
+impl From<&ast::LiteralPolicy> for models::Policy {
     fn from(v: &ast::LiteralPolicy) -> Self {
         Self {
             template_id: v.template_id().as_ref().to_string(),
             link_id: if v.is_static() {
-                String::new()
+                None
             } else {
-                v.id().as_ref().to_string()
+                Some(v.id().as_ref().to_string())
             },
-            link_id_specified: !v.is_static(),
+            is_template_link: !v.is_static(),
             principal_euid: v
                 .value(&ast::SlotId::principal())
                 .map(models::EntityUid::from),
@@ -88,16 +88,16 @@ impl From<&ast::LiteralPolicy> for models::LiteralPolicy {
     }
 }
 
-impl From<&ast::Policy> for models::LiteralPolicy {
+impl From<&ast::Policy> for models::Policy {
     fn from(v: &ast::Policy) -> Self {
         Self {
             template_id: v.template().id().as_ref().to_string(),
             link_id: if v.is_static() {
-                String::new()
+                None
             } else {
-                v.id().as_ref().to_string()
+                Some(v.id().as_ref().to_string())
             },
-            link_id_specified: !v.is_static(),
+            is_template_link: !v.is_static(),
             principal_euid: v
                 .env()
                 .get(&ast::SlotId::principal())
@@ -128,7 +128,10 @@ impl From<&models::TemplateBody> for ast::TemplateBody {
                 .map(|(key, value)| {
                     (
                         ast::AnyId::from_normalized_str(key).unwrap(),
-                        ast::Annotation::from(value),
+                        ast::Annotation {
+                            val: value.into(),
+                            loc: None,
+                        },
                     )
                 })
                 .collect(),
@@ -159,18 +162,22 @@ impl From<&models::TemplateBody> for ast::TemplateBody {
 
 impl From<&ast::TemplateBody> for models::TemplateBody {
     fn from(v: &ast::TemplateBody) -> Self {
-        let annotations: HashMap<String, models::Annotation> = v
+        let annotations: HashMap<String, String> = v
             .annotations()
-            .map(|(key, value)| (String::from(key.as_ref()), models::Annotation::from(value)))
+            .map(|(key, value)| (key.as_ref().into(), value.as_ref().into()))
             .collect();
 
         Self {
             id: v.id().as_ref().to_string(),
             annotations,
             effect: models::Effect::from(&v.effect()).into(),
-            principal_constraint: Some(models::PrincipalConstraint::from(v.principal_constraint())),
+            principal_constraint: Some(models::PrincipalOrResourceConstraint::from(
+                v.principal_constraint(),
+            )),
             action_constraint: Some(models::ActionConstraint::from(v.action_constraint())),
-            resource_constraint: Some(models::ResourceConstraint::from(v.resource_constraint())),
+            resource_constraint: Some(models::PrincipalOrResourceConstraint::from(
+                v.resource_constraint(),
+            )),
             non_scope_constraints: Some(models::Expr::from(v.non_scope_constraints())),
         }
     }
@@ -182,43 +189,31 @@ impl From<&ast::Template> for models::TemplateBody {
     }
 }
 
-impl From<&models::PrincipalConstraint> for ast::PrincipalConstraint {
+impl From<&models::PrincipalOrResourceConstraint> for ast::PrincipalConstraint {
     // PANIC SAFETY: experimental feature
     #[allow(clippy::expect_used)]
-    fn from(v: &models::PrincipalConstraint) -> Self {
-        Self::new(ast::PrincipalOrResourceConstraint::from(
-            v.constraint
-                .as_ref()
-                .expect("constraint field should exist"),
-        ))
+    fn from(v: &models::PrincipalOrResourceConstraint) -> Self {
+        Self::new(ast::PrincipalOrResourceConstraint::from(v))
     }
 }
 
-impl From<&ast::PrincipalConstraint> for models::PrincipalConstraint {
+impl From<&ast::PrincipalConstraint> for models::PrincipalOrResourceConstraint {
     fn from(v: &ast::PrincipalConstraint) -> Self {
-        Self {
-            constraint: Some(models::PrincipalOrResourceConstraint::from(v.as_inner())),
-        }
+        models::PrincipalOrResourceConstraint::from(v.as_inner())
     }
 }
 
-impl From<&models::ResourceConstraint> for ast::ResourceConstraint {
+impl From<&models::PrincipalOrResourceConstraint> for ast::ResourceConstraint {
     // PANIC SAFETY: experimental feature
     #[allow(clippy::expect_used)]
-    fn from(v: &models::ResourceConstraint) -> Self {
-        Self::new(ast::PrincipalOrResourceConstraint::from(
-            v.constraint
-                .as_ref()
-                .expect("constraint field should exist"),
-        ))
+    fn from(v: &models::PrincipalOrResourceConstraint) -> Self {
+        Self::new(ast::PrincipalOrResourceConstraint::from(v))
     }
 }
 
-impl From<&ast::ResourceConstraint> for models::ResourceConstraint {
+impl From<&ast::ResourceConstraint> for models::PrincipalOrResourceConstraint {
     fn from(v: &ast::ResourceConstraint) -> Self {
-        Self {
-            constraint: Some(models::PrincipalOrResourceConstraint::from(v.as_inner())),
-        }
+        models::PrincipalOrResourceConstraint::from(v.as_inner())
     }
 }
 
@@ -227,11 +222,11 @@ impl From<&models::EntityReference> for ast::EntityReference {
     #[allow(clippy::expect_used)]
     fn from(v: &models::EntityReference) -> Self {
         match v.data.as_ref().expect("data field should exist") {
-            models::entity_reference::Data::Ty(ty) => {
-                match models::entity_reference::Ty::try_from(ty.to_owned())
+            models::entity_reference::Data::Slot(slot) => {
+                match models::entity_reference::Slot::try_from(*slot)
                     .expect("decode should succeed")
                 {
-                    models::entity_reference::Ty::Slot => ast::EntityReference::Slot(None),
+                    models::entity_reference::Slot::Unit => ast::EntityReference::Slot(None),
                 }
             }
             models::entity_reference::Data::Euid(euid) => {
@@ -250,8 +245,8 @@ impl From<&ast::EntityReference> for models::EntityReference {
                 )),
             },
             ast::EntityReference::Slot(_) => Self {
-                data: Some(models::entity_reference::Data::Ty(
-                    models::entity_reference::Ty::Slot.into(),
+                data: Some(models::entity_reference::Data::Slot(
+                    models::entity_reference::Slot::Unit.into(),
                 )),
             },
         }
@@ -263,11 +258,11 @@ impl From<&models::PrincipalOrResourceConstraint> for ast::PrincipalOrResourceCo
     #[allow(clippy::expect_used)]
     fn from(v: &models::PrincipalOrResourceConstraint) -> Self {
         match v.data.as_ref().expect("data field should exist") {
-            models::principal_or_resource_constraint::Data::Ty(ty) => {
-                match models::principal_or_resource_constraint::Ty::try_from(ty.to_owned())
+            models::principal_or_resource_constraint::Data::Any(unit) => {
+                match models::principal_or_resource_constraint::Any::try_from(*unit)
                     .expect("decode should succeed")
                 {
-                    models::principal_or_resource_constraint::Ty::Any => {
+                    models::principal_or_resource_constraint::Any::Unit => {
                         ast::PrincipalOrResourceConstraint::Any
                     }
                 }
@@ -284,12 +279,22 @@ impl From<&models::PrincipalOrResourceConstraint> for ast::PrincipalOrResourceCo
             }
             models::principal_or_resource_constraint::Data::Is(msg) => {
                 ast::PrincipalOrResourceConstraint::Is(
-                    ast::EntityType::from(msg.et.as_ref().expect("et field should exist")).into(),
+                    ast::EntityType::from(
+                        msg.entity_type
+                            .as_ref()
+                            .expect("entity_type field should exist"),
+                    )
+                    .into(),
                 )
             }
             models::principal_or_resource_constraint::Data::IsIn(msg) => {
                 ast::PrincipalOrResourceConstraint::IsIn(
-                    ast::EntityType::from(msg.et.as_ref().expect("et field should exist")).into(),
+                    ast::EntityType::from(
+                        msg.entity_type
+                            .as_ref()
+                            .expect("entity_type field should exist"),
+                    )
+                    .into(),
                     ast::EntityReference::from(msg.er.as_ref().expect("er field should exist")),
                 )
             }
@@ -301,8 +306,8 @@ impl From<&ast::PrincipalOrResourceConstraint> for models::PrincipalOrResourceCo
     fn from(v: &ast::PrincipalOrResourceConstraint) -> Self {
         match v {
             ast::PrincipalOrResourceConstraint::Any => Self {
-                data: Some(models::principal_or_resource_constraint::Data::Ty(
-                    models::principal_or_resource_constraint::Ty::Any.into(),
+                data: Some(models::principal_or_resource_constraint::Data::Any(
+                    models::principal_or_resource_constraint::Any::Unit.into(),
                 )),
             },
             ast::PrincipalOrResourceConstraint::In(er) => Self {
@@ -322,7 +327,7 @@ impl From<&ast::PrincipalOrResourceConstraint> for models::PrincipalOrResourceCo
             ast::PrincipalOrResourceConstraint::Is(na) => Self {
                 data: Some(models::principal_or_resource_constraint::Data::Is(
                     models::principal_or_resource_constraint::IsMessage {
-                        et: Some(models::EntityType::from(na.as_ref())),
+                        entity_type: Some(models::Name::from(na.as_ref())),
                     },
                 )),
             },
@@ -330,7 +335,7 @@ impl From<&ast::PrincipalOrResourceConstraint> for models::PrincipalOrResourceCo
                 data: Some(models::principal_or_resource_constraint::Data::IsIn(
                     models::principal_or_resource_constraint::IsInMessage {
                         er: Some(models::EntityReference::from(er)),
-                        et: Some(models::EntityType::from(na.as_ref())),
+                        entity_type: Some(models::Name::from(na.as_ref())),
                     },
                 )),
             },
@@ -343,11 +348,11 @@ impl From<&models::ActionConstraint> for ast::ActionConstraint {
     #[allow(clippy::expect_used)]
     fn from(v: &models::ActionConstraint) -> Self {
         match v.data.as_ref().expect("data.as_ref()") {
-            models::action_constraint::Data::Ty(ty) => {
-                match models::action_constraint::Ty::try_from(ty.to_owned())
+            models::action_constraint::Data::Any(unit) => {
+                match models::action_constraint::Any::try_from(*unit)
                     .expect("decode should succeed")
                 {
-                    models::action_constraint::Ty::Any => ast::ActionConstraint::Any,
+                    models::action_constraint::Any::Unit => ast::ActionConstraint::Any,
                 }
             }
             models::action_constraint::Data::In(msg) => ast::ActionConstraint::In(
@@ -367,8 +372,8 @@ impl From<&ast::ActionConstraint> for models::ActionConstraint {
     fn from(v: &ast::ActionConstraint) -> Self {
         match v {
             ast::ActionConstraint::Any => Self {
-                data: Some(models::action_constraint::Data::Ty(
-                    models::action_constraint::Ty::Any.into(),
+                data: Some(models::action_constraint::Data::Any(
+                    models::action_constraint::Any::Unit.into(),
                 )),
             },
             ast::ActionConstraint::In(euids) => {
@@ -411,8 +416,8 @@ impl From<&ast::Effect> for models::Effect {
     }
 }
 
-impl From<&models::LiteralPolicySet> for ast::LiteralPolicySet {
-    fn from(v: &models::LiteralPolicySet) -> Self {
+impl From<&models::PolicySet> for ast::LiteralPolicySet {
+    fn from(v: &models::PolicySet) -> Self {
         let templates = v.templates.iter().map(|(key, value)| {
             (
                 ast::PolicyID::from_string(key),
@@ -431,7 +436,7 @@ impl From<&models::LiteralPolicySet> for ast::LiteralPolicySet {
     }
 }
 
-impl From<&ast::LiteralPolicySet> for models::LiteralPolicySet {
+impl From<&ast::LiteralPolicySet> for models::PolicySet {
     fn from(v: &ast::LiteralPolicySet) -> Self {
         let templates = v
             .templates()
@@ -447,7 +452,7 @@ impl From<&ast::LiteralPolicySet> for models::LiteralPolicySet {
             .map(|policy| {
                 (
                     String::from(policy.id().as_ref()),
-                    models::LiteralPolicy::from(policy),
+                    models::Policy::from(policy),
                 )
             })
             .collect();
@@ -456,18 +461,18 @@ impl From<&ast::LiteralPolicySet> for models::LiteralPolicySet {
     }
 }
 
-impl From<&ast::PolicySet> for models::LiteralPolicySet {
+impl From<&ast::PolicySet> for models::PolicySet {
     fn from(v: &ast::PolicySet) -> Self {
         let templates: HashMap<String, models::TemplateBody> = v
             .all_templates()
             .map(|t| (String::from(t.id().as_ref()), models::TemplateBody::from(t)))
             .collect();
-        let links: HashMap<String, models::LiteralPolicy> = v
+        let links: HashMap<String, models::Policy> = v
             .policies()
             .map(|policy| {
                 (
                     String::from(policy.id().as_ref()),
-                    models::LiteralPolicy::from(policy),
+                    models::Policy::from(policy),
                 )
             })
             .collect();
@@ -476,9 +481,9 @@ impl From<&ast::PolicySet> for models::LiteralPolicySet {
     }
 }
 
-impl TryFrom<&models::LiteralPolicySet> for ast::PolicySet {
+impl TryFrom<&models::PolicySet> for ast::PolicySet {
     type Error = ast::ReificationError;
-    fn try_from(pset: &models::LiteralPolicySet) -> Result<Self, Self::Error> {
+    fn try_from(pset: &models::PolicySet) -> Result<Self, Self::Error> {
         ast::PolicySet::try_from(ast::LiteralPolicySet::from(pset))
     }
 }
@@ -495,19 +500,11 @@ mod test {
             val: "".into(),
             loc: None,
         };
-        assert_eq!(
-            annotation1,
-            ast::Annotation::from(&models::Annotation::from(&annotation1))
-        );
 
         let annotation2 = ast::Annotation {
             val: "Hello World".into(),
             loc: None,
         };
-        assert_eq!(
-            annotation2,
-            ast::Annotation::from(&models::Annotation::from(&annotation2))
-        );
 
         assert_eq!(
             ast::Effect::Permit,
@@ -594,11 +591,11 @@ mod test {
         let rc = ast::ResourceConstraint::new(prc3);
         assert_eq!(
             pc,
-            ast::PrincipalConstraint::from(&models::PrincipalConstraint::from(&pc))
+            ast::PrincipalConstraint::from(&models::PrincipalOrResourceConstraint::from(&pc))
         );
         assert_eq!(
             rc,
-            ast::ResourceConstraint::from(&models::ResourceConstraint::from(&rc))
+            ast::ResourceConstraint::from(&models::PrincipalOrResourceConstraint::from(&rc))
         );
 
         assert_eq!(
@@ -613,10 +610,16 @@ mod test {
         let tb = ast::TemplateBody::new(
             ast::PolicyID::from_string("template"),
             None,
-            ast::Annotations::from_iter([(
-                ast::AnyId::from_normalized_str("read").unwrap(),
-                annotation1,
-            )]),
+            ast::Annotations::from_iter([
+                (
+                    ast::AnyId::from_normalized_str("read").unwrap(),
+                    annotation1,
+                ),
+                (
+                    ast::AnyId::from_normalized_str("write").unwrap(),
+                    annotation2,
+                ),
+            ]),
             ast::Effect::Permit,
             pc.clone(),
             ac1.clone(),
@@ -638,7 +641,7 @@ mod test {
         );
         assert_eq!(
             policy,
-            ast::LiteralPolicy::from(&models::LiteralPolicy::from(&policy))
+            ast::LiteralPolicy::from(&models::Policy::from(&policy))
         );
 
         let tb = ast::TemplateBody::new(
@@ -666,7 +669,7 @@ mod test {
         );
         assert_eq!(
             policy,
-            ast::LiteralPolicy::from(&models::LiteralPolicy::from(&policy))
+            ast::LiteralPolicy::from(&models::Policy::from(&policy))
         );
     }
 
@@ -725,12 +728,12 @@ mod test {
             )]),
         )
         .unwrap();
-        let lps = models::LiteralPolicySet::from(&ps);
-        let lps_roundtrip = models::LiteralPolicySet::from(&ast::LiteralPolicySet::from(&lps));
+        let mps = models::PolicySet::from(&ps);
+        let mps_roundtrip = models::PolicySet::from(&ast::LiteralPolicySet::from(&mps));
 
         // Can't compare LiteralPolicySets directly, so we compare their fields
-        assert_eq!(lps.templates, lps_roundtrip.templates);
-        assert_eq!(lps.links, lps_roundtrip.links);
+        assert_eq!(mps.templates, mps_roundtrip.templates);
+        assert_eq!(mps.links, mps_roundtrip.links);
     }
 
     #[test]
@@ -788,11 +791,11 @@ mod test {
             )]),
         )
         .unwrap();
-        let lps = models::LiteralPolicySet::from(&ps);
-        let lps_roundtrip = models::LiteralPolicySet::from(&ast::LiteralPolicySet::from(&lps));
+        let mps = models::PolicySet::from(&ps);
+        let mps_roundtrip = models::PolicySet::from(&ast::LiteralPolicySet::from(&mps));
 
-        // Can't compare LiteralPolicySets directly, so we compare their fields
-        assert_eq!(lps.templates, lps_roundtrip.templates);
-        assert_eq!(lps.links, lps_roundtrip.links);
+        // Can't compare `models::PolicySet` directly, so we compare their fields
+        assert_eq!(mps.templates, mps_roundtrip.templates);
+        assert_eq!(mps.links, mps_roundtrip.links);
     }
 }
diff --git a/cedar-policy/src/proto/validator.rs b/cedar-policy/src/proto/validator.rs
index 7680aeafb..3d97c6004 100644
--- a/cedar-policy/src/proto/validator.rs
+++ b/cedar-policy/src/proto/validator.rs
@@ -17,49 +17,49 @@
 #![allow(clippy::use_self)]
 
 use super::models;
-use cedar_policy_core::{ast, evaluator, extensions};
+use cedar_policy_core::ast;
 use cedar_policy_validator::types;
 use nonempty::NonEmpty;
 use smol_str::SmolStr;
-use std::collections::HashMap;
+use std::collections::{BTreeMap, HashMap};
 
-impl From<&cedar_policy_validator::ValidatorSchema> for models::ValidatorSchema {
+impl From<&cedar_policy_validator::ValidatorSchema> for models::Schema {
     fn from(v: &cedar_policy_validator::ValidatorSchema) -> Self {
         Self {
-            entity_types: v
+            entity_decls: v
                 .entity_types()
-                .map(|ety| models::EntityTypeWithTypesMap {
-                    key: Some(models::EntityType::from(ety.name())),
-                    value: Some(models::ValidatorEntityType::from(ety)),
+                .map(|ety| models::EntityTypeToEntityDeclMap {
+                    key: Some(models::Name::from(ety.name())),
+                    value: Some(models::EntityDecl::from(ety)),
                 })
                 .collect(),
-            action_ids: v
+            action_decls: v
                 .action_ids()
-                .map(|id| models::EntityUidWithActionIdsMap {
+                .map(|id| models::EntityUidToActionDeclMap {
                     key: Some(models::EntityUid::from(id.name())),
-                    value: Some(models::ValidatorActionId::from(id)),
+                    value: Some(models::ActionDecl::from(id)),
                 })
                 .collect(),
         }
     }
 }
 
-impl From<&models::ValidatorSchema> for cedar_policy_validator::ValidatorSchema {
+impl From<&models::Schema> for cedar_policy_validator::ValidatorSchema {
     // PANIC SAFETY: experimental feature
     #[allow(clippy::expect_used)]
-    fn from(v: &models::ValidatorSchema) -> Self {
+    fn from(v: &models::Schema) -> Self {
         Self::new(
-            v.entity_types
+            v.entity_decls
                 .iter()
-                .map(|models::EntityTypeWithTypesMap { key, value }| {
+                .map(|models::EntityTypeToEntityDeclMap { key, value }| {
                     let key = key.as_ref().expect("key field should exist");
                     let value = value.as_ref().expect("value field should exist");
                     assert_eq!(key, value.name.as_ref().expect("name field should exist"));
                     cedar_policy_validator::ValidatorEntityType::from(value)
                 }),
-            v.action_ids
+            v.action_decls
                 .iter()
-                .map(|models::EntityUidWithActionIdsMap { key, value }| {
+                .map(|models::EntityUidToActionDeclMap { key, value }| {
                     let key = key.as_ref().expect("key field should exist");
                     let value = value.as_ref().expect("value field should exist");
                     assert_eq!(key, value.name.as_ref().expect("name field should exist"));
@@ -95,87 +95,53 @@ impl From<&models::ValidationMode> for cedar_policy_validator::ValidationMode {
     }
 }
 
-impl From<&cedar_policy_validator::ValidatorActionId> for models::ValidatorActionId {
+impl From<&cedar_policy_validator::ValidatorActionId> for models::ActionDecl {
     fn from(v: &cedar_policy_validator::ValidatorActionId) -> Self {
+        debug_assert_eq!(
+            v.attribute_types().keys().collect::<Vec<&SmolStr>>(),
+            Vec::<&SmolStr>::new(),
+            "action attributes are not currently supported in protobuf"
+        );
+        debug_assert_eq!(
+            v.attributes().collect::<Vec<_>>(),
+            vec![],
+            "action attributes are not currently supported in protobuf"
+        );
         Self {
             name: Some(models::EntityUid::from(v.name())),
-            applies_to: Some(models::ValidatorApplySpec {
-                principal_apply_spec: v
-                    .applies_to_principals()
-                    .map(models::EntityType::from)
-                    .collect(),
-                resource_apply_spec: v
-                    .applies_to_resources()
-                    .map(models::EntityType::from)
-                    .collect(),
-            }),
+            principal_types: v.applies_to_principals().map(models::Name::from).collect(),
+            resource_types: v.applies_to_resources().map(models::Name::from).collect(),
             descendants: v.descendants().map(models::EntityUid::from).collect(),
             context: Some(models::Type::from(v.context())),
-            attribute_types: Some(models::Attributes::from(v.attribute_types())),
-            attributes: v
-                .attributes()
-                .map(|(k, v)| {
-                    let value =
-                        models::Expr::from(&ast::Expr::from(ast::PartialValue::from(v.to_owned())));
-                    (k.to_string(), value)
-                })
-                .collect(),
         }
     }
 }
 
-impl From<&models::ValidatorActionId> for cedar_policy_validator::ValidatorActionId {
+impl From<&models::ActionDecl> for cedar_policy_validator::ValidatorActionId {
     // PANIC SAFETY: experimental feature
     #[allow(clippy::expect_used)]
-    fn from(v: &models::ValidatorActionId) -> Self {
-        let extensions_none = extensions::Extensions::none();
-        let eval = evaluator::RestrictedEvaluator::new(extensions_none);
+    fn from(v: &models::ActionDecl) -> Self {
         Self::new(
             ast::EntityUID::from(v.name.as_ref().expect("name field should exist")),
-            v.applies_to
-                .as_ref()
-                .expect("applies_to field should exist")
-                .principal_apply_spec
-                .iter()
-                .map(ast::EntityType::from),
-            v.applies_to
-                .as_ref()
-                .expect("applies_to field should exist")
-                .resource_apply_spec
-                .iter()
-                .map(ast::EntityType::from),
+            v.principal_types.iter().map(ast::EntityType::from),
+            v.resource_types.iter().map(ast::EntityType::from),
             v.descendants.iter().map(ast::EntityUID::from),
             types::Type::from(v.context.as_ref().expect("context field should exist")),
-            types::Attributes::from(
-                v.attribute_types
-                    .as_ref()
-                    .expect("attribute_types field should exist"),
-            ),
-            v.attributes
-                .iter()
-                .map(|(k, v)| {
-                    let pval = eval
-                        .partial_interpret(
-                            ast::BorrowedRestrictedExpr::new(&ast::Expr::from(v))
-                                .expect("RestrictedExpr"),
-                        )
-                        .expect("interpret on RestrictedExpr");
-                    (k.into(), pval.into())
-                })
-                .collect(),
+            // protobuf formats do not include action attributes, so we
+            // translate into a `ValidatorActionId` with no action attributes
+            types::Attributes::with_attributes([]),
+            BTreeMap::new(),
         )
     }
 }
 
-impl From<&cedar_policy_validator::ValidatorEntityType> for models::ValidatorEntityType {
+impl From<&cedar_policy_validator::ValidatorEntityType> for models::EntityDecl {
     fn from(v: &cedar_policy_validator::ValidatorEntityType) -> Self {
-        let name = Some(models::EntityType::from(v.name()));
-        let descendants = v.descendants.iter().map(models::EntityType::from).collect();
-        let attributes = Some(models::Attributes::from(v.attributes()));
+        let name = Some(models::Name::from(v.name()));
+        let descendants = v.descendants.iter().map(models::Name::from).collect();
+        let attributes = attributes_to_model(v.attributes());
         let open_attributes = models::OpenTag::from(&v.open_attributes()).into();
-        let tags = v.tag_type().map(|tags| models::Tag {
-            optional_type: Some(models::Type::from(tags)),
-        });
+        let tags = v.tag_type().map(models::Type::from);
         match &v.kind {
             cedar_policy_validator::ValidatorEntityTypeKind::Standard(_) => Self {
                 name,
@@ -197,10 +163,10 @@ impl From<&cedar_policy_validator::ValidatorEntityType> for models::ValidatorEnt
     }
 }
 
-impl From<&models::ValidatorEntityType> for cedar_policy_validator::ValidatorEntityType {
+impl From<&models::EntityDecl> for cedar_policy_validator::ValidatorEntityType {
     // PANIC SAFETY: experimental feature
     #[allow(clippy::expect_used)]
-    fn from(v: &models::ValidatorEntityType) -> Self {
+    fn from(v: &models::EntityDecl) -> Self {
         let name = ast::EntityType::from(v.name.as_ref().expect("name field should exist"));
         let descendants = v.descendants.iter().map(ast::EntityType::from);
         match NonEmpty::collect(v.enum_choices.iter().map(SmolStr::from)) {
@@ -208,38 +174,17 @@ impl From<&models::ValidatorEntityType> for cedar_policy_validator::ValidatorEnt
             None => Self::new_standard(
                 name,
                 descendants,
-                types::Attributes::from(
-                    v.attributes
-                        .as_ref()
-                        .expect("attributes field should exist"),
-                ),
+                model_to_attributes(&v.attributes),
                 types::OpenTag::from(
                     &models::OpenTag::try_from(v.open_attributes).expect("decode should succeed"),
                 ),
-                v.tags
-                    .as_ref()
-                    .and_then(|tags| tags.optional_type.as_ref().map(types::Type::from)),
+                v.tags.as_ref().map(types::Type::from),
             ),
             Some(enum_choices) => {
                 // `enum_choices` is not empty, so `v` represents an enumerated entity type.
-                if let Some(vec) = &v.attributes {
-                    // enumerated entity types must have no attributes
-                    assert_eq!(
-                        vec,
-                        &models::Attributes {
-                            attrs: HashMap::new()
-                        }
-                    );
-                }
-                if let Some(tag) = &v.tags {
-                    // enumerated entity types must have no tags
-                    assert_eq!(
-                        tag,
-                        &models::Tag {
-                            optional_type: None
-                        }
-                    );
-                }
+                // enumerated entity types must have no attributes or tags.
+                assert_eq!(&v.attributes, &HashMap::new());
+                assert_eq!(&v.tags, &None);
                 Self::new_enum(name, descendants, enum_choices)
             }
         }
@@ -321,25 +266,14 @@ impl From<&types::Type> for models::Type {
     }
 }
 
-impl From<&models::Attributes> for types::Attributes {
-    fn from(v: &models::Attributes) -> Self {
-        Self::with_attributes(
-            v.attrs
-                .iter()
-                .map(|(k, v)| (k.into(), types::AttributeType::from(v))),
-        )
-    }
+fn model_to_attributes(v: &HashMap<String, models::AttributeType>) -> types::Attributes {
+    types::Attributes::with_attributes(v.iter().map(|(k, v)| (k.into(), v.into())))
 }
 
-impl From<&types::Attributes> for models::Attributes {
-    fn from(v: &types::Attributes) -> Self {
-        Self {
-            attrs: v
-                .iter()
-                .map(|(k, v)| (k.to_string(), models::AttributeType::from(v)))
-                .collect(),
-        }
-    }
+fn attributes_to_model(v: &types::Attributes) -> HashMap<String, models::AttributeType> {
+    v.iter()
+        .map(|(k, v)| (k.to_string(), models::AttributeType::from(v)))
+        .collect()
 }
 
 impl From<&models::OpenTag> for types::OpenTag {
@@ -365,40 +299,25 @@ impl From<&models::EntityRecordKind> for types::EntityRecordKind {
     #[allow(clippy::expect_used)]
     fn from(v: &models::EntityRecordKind) -> Self {
         match v.data.as_ref().expect("data field should exist") {
-            models::entity_record_kind::Data::Ty(ty) => {
-                match models::entity_record_kind::Ty::try_from(ty.to_owned())
+            models::entity_record_kind::Data::AnyEntity(unit) => {
+                match models::entity_record_kind::AnyEntity::try_from(*unit)
                     .expect("decode should succeed")
                 {
-                    models::entity_record_kind::Ty::AnyEntity => Self::AnyEntity,
+                    models::entity_record_kind::AnyEntity::Unit => Self::AnyEntity,
                 }
             }
-            models::entity_record_kind::Data::Record(p_record) => Self::Record {
-                attrs: types::Attributes::from(
-                    p_record.attrs.as_ref().expect("attrs field should exist"),
-                ),
+            models::entity_record_kind::Data::Record(r) => Self::Record {
+                attrs: model_to_attributes(&r.attrs),
                 open_attributes: types::OpenTag::from(
-                    &models::OpenTag::try_from(p_record.open_attributes)
-                        .expect("decode should succeed"),
+                    &models::OpenTag::try_from(r.open_attributes).expect("decode should succeed"),
                 ),
             },
-            models::entity_record_kind::Data::Entity(p_entity) => {
-                Self::Entity(types::EntityLUB::single_entity(ast::EntityType::from(
-                    p_entity.e.as_ref().expect("e field should exist"),
-                )))
+            models::entity_record_kind::Data::Entity(name) => {
+                Self::Entity(types::EntityLUB::single_entity(ast::EntityType::from(name)))
             }
-            models::entity_record_kind::Data::ActionEntity(p_action_entity) => Self::ActionEntity {
-                name: ast::EntityType::from(
-                    p_action_entity
-                        .name
-                        .as_ref()
-                        .expect("name field should exist"),
-                ),
-                attrs: types::Attributes::from(
-                    p_action_entity
-                        .attrs
-                        .as_ref()
-                        .expect("attrs field should exist"),
-                ),
+            models::entity_record_kind::Data::ActionEntity(act) => Self::ActionEntity {
+                name: ast::EntityType::from(act.name.as_ref().expect("name field should exist")),
+                attrs: model_to_attributes(&act.attrs),
             },
         }
     }
@@ -413,26 +332,24 @@ impl From<&types::EntityRecordKind> for models::EntityRecordKind {
                 attrs,
                 open_attributes,
             } => models::entity_record_kind::Data::Record(models::entity_record_kind::Record {
-                attrs: Some(models::Attributes::from(attrs)),
+                attrs: attributes_to_model(attrs),
                 open_attributes: models::OpenTag::from(open_attributes).into(),
             }),
-            types::EntityRecordKind::AnyEntity => models::entity_record_kind::Data::Ty(
-                models::entity_record_kind::Ty::AnyEntity.into(),
+            types::EntityRecordKind::AnyEntity => models::entity_record_kind::Data::AnyEntity(
+                models::entity_record_kind::AnyEntity::Unit.into(),
             ),
             types::EntityRecordKind::Entity(e) => {
-                models::entity_record_kind::Data::Entity(models::entity_record_kind::Entity {
-                    e: Some(models::EntityType::from(
-                        &e.clone()
-                            .into_single_entity()
-                            .expect("will be single EntityType"),
-                    )),
-                })
+                models::entity_record_kind::Data::Entity(models::Name::from(
+                    &e.clone()
+                        .into_single_entity()
+                        .expect("will be single EntityType"),
+                ))
             }
             types::EntityRecordKind::ActionEntity { name, attrs } => {
                 models::entity_record_kind::Data::ActionEntity(
                     models::entity_record_kind::ActionEntity {
-                        name: Some(models::EntityType::from(name)),
-                        attrs: Some(models::Attributes::from(attrs)),
+                        name: Some(models::Name::from(name)),
+                        attrs: attributes_to_model(attrs),
                     },
                 )
             }