git-ents.gitmain
⌘K
foforge

crates/forge/ents-forge/src/review/entity.rs

entity.rs338 lines · 13.1 KB · rusthistorycomment on this file
1//! The Review entity: a verdict plus a context — the id of the most
2//! recently reviewed commit, a verdict, and a body.
3//!
4//! Spec coverage: `model.review`.
5
6use ents_attrs as ents;
7use facet::Facet;
8use gix_hash::ObjectId;
9
10/// A review's lifecycle state (`model.review`): whether the reviewer's
11/// verdict still stands (`Active`) or the reviewer has retracted it
12/// (`Withdrawn`). Withdrawal is append-only — [`super::command::withdraw`]
13/// writes a new [`Review`] entity carrying this variant onto the *same*
14/// ref chain rather than deleting anything, so a withdrawn verdict remains
15/// in `refs/meta/reviews/<target>/<member>`'s history: the chain is the
16/// audit trail. Web aggregate views (the `/reviews` list, a commit's own
17/// reviews section) filter `Withdrawn` rows out of what they render, but
18/// nothing here or in `super::command` ever removes the ref, the object,
19/// or an earlier commit naming `Active`.
20///
21/// `Active` is this type's [`Default`] and the [`Review`] field carrying it
22/// is `#[facet(default)]` for exactly one reason: every review tree written
23/// before this variant existed has no `state` entry at all, and must still
24/// read back as a plain, unretracted review rather than fail to decode
25/// (backward compatibility with every review recorded before this change).
26///
27/// Parses from and renders as its kebab-case convention names (`active`,
28/// `withdrawn`), the same convention [`Verdict`] follows.
29///
30/// # Examples
31///
32/// ```
33/// use ents_forge::review::ReviewState;
34///
35/// let state: ReviewState = "withdrawn".parse().expect("known state");
36/// assert_eq!(state, ReviewState::Withdrawn);
37/// assert_eq!(state.to_string(), "withdrawn");
38/// assert_eq!(ReviewState::default(), ReviewState::Active);
39/// ```
40// @relation(model.review, meta-ref.typed-tree, scope=type)
41#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Facet)]
42#[repr(u8)]
43pub enum ReviewState {
44 /// The reviewer's verdict still stands.
45 #[default]
46 Active,
47 /// The reviewer has retracted this review; the verdict and body stay
48 /// in history, unread by aggregate views.
49 Withdrawn,
50}
51
52impl std::str::FromStr for ReviewState {
53 type Err = crate::Error;
54
55 fn from_str(text: &str) -> Result<Self, Self::Err> {
56 match text {
57 "active" => Ok(Self::Active),
58 "withdrawn" => Ok(Self::Withdrawn),
59 other => Err(crate::Error::InvalidArgument(format!(
60 "unknown review state {other:?}: expected active or withdrawn"
61 ))),
62 }
63 }
64}
65
66impl std::fmt::Display for ReviewState {
67 fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
68 f.write_str(match self {
69 Self::Active => "active",
70 Self::Withdrawn => "withdrawn",
71 })
72 }
73}
74
75/// A review's verdict (`model.review`): a hard enum, unlike issue and
76/// comment states — a verdict gates decisions, so its vocabulary is
77/// platform, not schema.
78///
79/// Parses from and renders as its kebab-case convention names
80/// (`approve`, `request-changes`, `comment`), the same strings every
81/// surface shows.
82///
83/// # Examples
84///
85/// ```
86/// use ents_forge::review::Verdict;
87///
88/// let verdict: Verdict = "request-changes".parse().expect("known verdict");
89/// assert_eq!(verdict, Verdict::RequestChanges);
90/// assert_eq!(verdict.to_string(), "request-changes");
91/// assert!("needs-design-doc".parse::<Verdict>().is_err());
92/// ```
93// @relation(model.review, scope=type)
94#[derive(Debug, Clone, Copy, PartialEq, Eq, Facet)]
95#[repr(u8)]
96pub enum Verdict {
97 /// The reviewed content is accepted.
98 Approve,
99 /// The reviewed content needs changes before acceptance.
100 RequestChanges,
101 /// Judgment withheld: the review exists for its body and thread.
102 Comment,
103}
104
105impl std::str::FromStr for Verdict {
106 type Err = crate::Error;
107
108 fn from_str(text: &str) -> Result<Self, Self::Err> {
109 match text {
110 "approve" => Ok(Self::Approve),
111 "request-changes" => Ok(Self::RequestChanges),
112 "comment" => Ok(Self::Comment),
113 other => Err(crate::Error::InvalidArgument(format!(
114 "unknown verdict {other:?}: expected approve, request-changes, or comment"
115 ))),
116 }
117 }
118}
119
120impl std::fmt::Display for Verdict {
121 fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
122 f.write_str(match self {
123 Self::Approve => "approve",
124 Self::RequestChanges => "request-changes",
125 Self::Comment => "comment",
126 })
127 }
128}
129
130/// A verdict on a commit, plus a body (`model.review`).
131///
132/// Every review occupies exactly two refs: this entity's own tree at
133/// `refs/meta/reviews/<target>/<member>`, and a retention pin at
134/// `refs/meta/pins/reviews/<target>/<member>` anchoring the reviewed content
135/// itself (`model.review-pin`) — [`super::new`] writes both. `target` is
136/// the oid of the most recently reviewed commit, stored as a plain data
137/// field the same way [`ents_model::ResultRecord`]'s own `target` field
138/// stores the commit it judged: a `[u8; 20]` field plus a [`Review::target`]
139/// accessor, so reading it back never requires the pin ref — the pin
140/// anchors reachability, the entity describes what was reviewed. At
141/// genesis this field equals the refname's `<target>` segment and binds
142/// it (`meta-ref.identity-binding`); re-reviewing a descendant advances
143/// this field while the refname stays keyed by genesis
144/// (`model.review-pin`). The verdict is a hard [`Verdict`] enum — unlike
145/// the open state vocabularies on [`crate::Issue`] and
146/// [`crate::comment::Comment`], a verdict gates decisions, so its
147/// vocabulary is platform, not schema. Reviewer and
148/// timestamp come from the mutation commit chain rather than a stored
149/// field (`meta-ref.identity-binding`), so `Review` carries no author or
150/// timestamp field — the same omission [`crate::comment::Comment`] makes.
151/// A review's discussion is [`crate::comment::Comment`] entities naming
152/// the review as their context (`model.comment-context`); `Review` itself
153/// stores no list of its comments.
154///
155/// # Examples
156///
157/// ```
158/// use ents_forge::review::Review;
159///
160/// let target = gix_hash::ObjectId::from_hex(b"0123456789abcdef0123456789abcdef01234567")
161/// .expect("valid hex");
162/// let review = Review::new(target, ents_forge::review::Verdict::Approve, "looks good");
163/// let (root, store) = facet_git_tree::serialize(&review).expect("serialize");
164/// let back: Review = facet_git_tree::deserialize(&root, &store).expect("deserialize");
165/// assert_eq!(back, review);
166/// assert_eq!(back.target(), target);
167/// ```
168// @relation(model.review, meta-ref.identity-binding, meta-ref.typed-tree, model.extensibility, scope=file)
169#[derive(Debug, Clone, PartialEq, Eq, Facet)]
170pub struct Review {
171 #[facet(ents::head, ents::id)]
172 target: [u8; 20],
173 /// The review's verdict (`model.review`): a fixed [`Verdict`], not a
174 /// string.
175 #[facet(ents::head)]
176 pub verdict: Verdict,
177 /// The review's body text.
178 #[facet(ents::body)]
179 pub body: String,
180 /// Whether this review still stands or has been withdrawn
181 /// (`model.review`). `#[facet(default)]` so a tree written before this
182 /// field existed — no `state` entry at all — deserializes to
183 /// [`ReviewState::Active`] rather than failing to decode; every
184 /// existing `refs/meta/reviews/*` history predates this field and must
185 /// keep reading.
186 #[facet(default, ents::head)]
187 pub state: ReviewState,
188}
189
190impl Review {
191 /// Build a review of `target` carrying `verdict` and `body`
192 /// (`model.review`), initially [`ReviewState::Active`] — every review
193 /// starts active; only [`super::command::withdraw`] ever writes
194 /// [`ReviewState::Withdrawn`].
195 #[must_use]
196 pub fn new(target: ObjectId, verdict: Verdict, body: impl Into<String>) -> Self {
197 let mut bytes = [0u8; 20];
198 bytes.copy_from_slice(target.as_slice());
199 Self {
200 target: bytes,
201 verdict,
202 body: body.into(),
203 state: ReviewState::Active,
204 }
205 }
206
207 /// The id of the most recently reviewed commit (`model.review`):
208 /// reading this never requires the pin ref
209 /// (`refs/meta/pins/reviews/<target>/<member>`) — the pin anchors
210 /// reachability, the entity describes what was reviewed.
211 #[must_use]
212 pub fn target(&self) -> ObjectId {
213 ObjectId::from_bytes_or_panic(&self.target)
214 }
215
216 /// A copy of this review with its state advanced to
217 /// [`ReviewState::Withdrawn`], preserving `target`, `verdict`, and
218 /// `body` exactly (`model.review`): [`super::command::withdraw`] writes
219 /// this new entity onto the same ref chain the original review
220 /// occupies — append-only, so the prior [`Active`](ReviewState::Active)
221 /// commit stays reachable in history — rather than mutating anything
222 /// in place.
223 #[must_use]
224 pub fn withdrawn(&self) -> Self {
225 Self {
226 state: ReviewState::Withdrawn,
227 ..self.clone()
228 }
229 }
230}
231
232#[cfg(test)]
233mod tests {
234 #![allow(clippy::expect_used, reason = "unit test")]
235
236 use facet_git_tree::{deserialize, serialize};
237 use rstest::rstest;
238
239 use super::*;
240
241 #[rstest]
242 #[case::approve(Verdict::Approve)]
243 #[case::request_changes(Verdict::RequestChanges)]
244 #[case::comment(Verdict::Comment)]
245 // @relation(model.review, meta-ref.typed-tree, scope=function, role=Verifies)
246 fn review_round_trips_with_every_verdict(#[case] verdict: Verdict) {
247 let target =
248 ObjectId::from_hex(b"0123456789abcdef0123456789abcdef01234567").expect("valid hex");
249 let review = Review::new(target, verdict, "reviewed the change");
250 let (root, store) = serialize(&review).expect("serialize");
251 let back: Review = deserialize(&root, &store).expect("deserialize");
252 assert_eq!(back, review);
253 assert_eq!(back.target(), target);
254 }
255
256 #[rstest]
257 // @relation(model.review, scope=function, role=Verifies)
258 fn target_accessor_reflects_the_stored_bytes() {
259 let target =
260 ObjectId::from_hex(b"fedcba9876543210fedcba9876543210fedcba98").expect("valid hex");
261 let review = Review::new(target, Verdict::Approve, "");
262 assert_eq!(review.target(), target);
263 }
264
265 /// The exact shape a `Review` tree had before [`ReviewState`] existed —
266 /// `target`/`verdict`/`body` only, no `state` entry at all. Local to
267 /// this test: it stands in for every `refs/meta/reviews/<target>/*`
268 /// tree already recorded in a real repository before this change
269 /// landed.
270 #[derive(Facet)]
271 struct PreStateReview {
272 target: [u8; 20],
273 verdict: Verdict,
274 body: String,
275 }
276
277 #[rstest]
278 // @relation(model.review, meta-ref.typed-tree, scope=function, role=Verifies)
279 fn a_review_tree_written_before_state_existed_reads_back_as_active() {
280 let target =
281 ObjectId::from_hex(b"0123456789abcdef0123456789abcdef01234567").expect("valid hex");
282 let mut bytes = [0u8; 20];
283 bytes.copy_from_slice(target.as_slice());
284 let legacy = PreStateReview {
285 target: bytes,
286 verdict: Verdict::RequestChanges,
287 body: "reviewed before withdrawal existed".to_owned(),
288 };
289 let (root, store) = serialize(&legacy).expect("serialize the pre-state shape");
290 let back: Review =
291 deserialize(&root, &store).expect("today's Review must still decode a tree with no \
292 state entry");
293 assert_eq!(back.state, ReviewState::Active);
294 assert_eq!(back.verdict, Verdict::RequestChanges);
295 assert_eq!(back.body, "reviewed before withdrawal existed");
296 assert_eq!(back.target(), target);
297 }
298
299 #[rstest]
300 // @relation(model.review, scope=function, role=Verifies)
301 fn withdrawn_preserves_target_verdict_and_body_and_flips_only_state() {
302 let target =
303 ObjectId::from_hex(b"0123456789abcdef0123456789abcdef01234567").expect("valid hex");
304 let review = Review::new(target, Verdict::Approve, "looks good");
305 let withdrawn = review.withdrawn();
306
307 assert_eq!(withdrawn.state, ReviewState::Withdrawn);
308 assert_eq!(withdrawn.verdict, review.verdict);
309 assert_eq!(withdrawn.body, review.body);
310 assert_eq!(withdrawn.target(), review.target());
311
312 // Idempotent-friendly: withdrawing an already-withdrawn review is a
313 // no-op-ish re-write, not an error or a second distinct shape.
314 let withdrawn_again = withdrawn.withdrawn();
315 assert_eq!(withdrawn_again, withdrawn);
316 }
317
318 #[rstest]
319 #[case::active("active", ReviewState::Active)]
320 #[case::withdrawn("withdrawn", ReviewState::Withdrawn)]
321 // @relation(model.review, scope=function, role=Verifies)
322 fn review_state_parses_its_own_display_strings(
323 #[case] text: &str,
324 #[case] expected: ReviewState,
325 ) {
326 let parsed: ReviewState = text.parse().expect("known state");
327 assert_eq!(parsed, expected);
328 assert_eq!(parsed.to_string(), text);
329 }
330
331 #[rstest]
332 // @relation(model.review, scope=function, role=Verifies)
333 fn review_state_rejects_an_unknown_string() {
334 "revoked"
335 .parse::<ReviewState>()
336 .expect_err("not a known review state");
337 }
338}