model: drop the comment legacy-read fallback
commit
1ebf59fmodel: drop the comment legacy-read fallback
This is an experimental repository with no on-disk data to preserve, so a struct change is a clean break rather than a permanent back-compat code path: read a comment tree only as its current shape. Removes the LegacyComment shape, its fallback in read_comment, and the phase-7 read-back tests; meta-ref.migration’s forward rewrite still applies if data ever needs carrying.
model: read comment trees only as the current struct docs: make the phase-9 comment migration a clean break, not a legacy reader Assisted-by: Claude:claude-fable-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
docs/development-plan.adoc
@@ -220,9 +220,10 @@
| fable
| Comments become the universal conversational primitive (`model.comment`
broadened, `model.comment-state`, `model.comment-context`,
- `model.comment-thread`, `model.review`): the Comment struct's migration
- (`meta-ref.migration` — old trees must stay readable), working-tree
- capture and projection in `ents-anchor` (`anchor.working-tree`), the
+ `model.comment-thread`, `model.review`): the Comment struct broadens as
+ a clean break (experimental repo, no on-disk data to preserve, so no
+ back-compat reader), working-tree capture and projection in
+ `ents-anchor` (`anchor.working-tree`), the
Review entity plus its retention pin ref (`model.review-pin` —
`refs/meta/pins/reviews/<id>`, an empty-tree signed commit whose
parents include the reviewed commit; verify the gate's tip-signed and
@@ -306,8 +307,12 @@
(`lens.parity`), resolved through the CLI (`model.comment-state`),
and gone from the lens's next publish; an issue and a review each
round-trip CLI ↔ web with their threads rendered as context queries
- (`model.comment-context`, `model.review`); pre-migration comment refs
- written by phase-7 code still read back (`meta-ref.migration`).
+ (`model.comment-context`, `model.review`). This being an experimental
+ repository with no on-disk data to preserve, a struct change is a
+ clean break: entities read only as their current shape, and no
+ back-compat reader is kept — `meta-ref.migration`'s forward rewrite
+ applies whenever data actually needs carrying, not a permanent legacy
+ code path.
Backend conformance tests are shared suites written once against each
trait (`RefStore`, `EventSink`, `Executor`) and run per implementation —
crates/forge/ents-forge/tests/conversations.rs
@@ -1,10 +1,7 @@
//! Integration coverage for the comment command layer: the broadened
//! `model.comment` (aboutness refused at creation, `model.comment-state`
//! transitions, `model.comment-context`/`model.comment-thread`
-//! aggregation) and — with the most care, per this phase's plan row — the
-//! `meta-ref.migration` guarantee that comment trees written by phase-7
-//! code still read back, and are rewritten under the current struct only
-//! when mutated.
+//! aggregation).
#![allow(
clippy::expect_used,
@@ -16,23 +13,9 @@
use ents_forge::comment::{self, ListFilter, NewComment};
use ents_receive::{Identity, Mode, NullEventSink, TxResult};
-use ents_testutil::{Keypair, MemRefStore, ObjectStore, write_meta_entity};
-use facet_git_tree::RawTree;
-use gix_object::Write as _;
-use gix_ref_store::RefStoreRead as _;
+use ents_testutil::{Keypair, MemRefStore, ObjectStore};
use rstest::rstest;
-/// The exact struct phase-7 code declared for its Comment entity — the
-/// on-disk encoding `meta-ref.migration` requires the broadened reader to
-/// keep reading. Declared independently here (not imported) so this test
-/// pins the *storage shape*, not whatever the crate's internal legacy
-/// struct happens to be.
-#[derive(facet::Facet)]
-struct Phase7Comment {
- body: String,
- anchor: RawTree,
-}
-
/// A throwaway on-disk repository holding one committed file — the
/// content anchors capture against — alongside the in-memory ref/object
/// fixtures every library test uses.
@@ -117,145 +100,6 @@
assert_eq!(outcome.result, TxResult::Applied);
id
}
-
- /// Seed a comment ref exactly as phase-7 code wrote one: the bare
- /// `{body, anchor}` tree under `refs/meta/comments/<id>`.
- fn seed_phase7(&self, id: &str, body: &str) -> gix_hash::ObjectId {
- let anchor = ents_anchor::capture(
- &gix::open(self.path()).expect("opens"),
- "HEAD",
- "file.txt",
- None,
- )
- .expect("captures");
- let anchor_tree =
- facet_git_tree::serialize_into(&anchor, &self.objects).expect("serializes");
- let legacy = Phase7Comment {
- body: body.to_owned(),
- anchor: RawTree::new(anchor_tree),
- };
- let name = ents_model::namespace::comment_ref(id).expect("valid");
- write_meta_entity(&self.refs, &self.objects, name, &legacy, None, 500)
- }
-}
-
-// ---------------------------------------------------------------------
-// meta-ref.migration: phase-7 trees still read; mutation rewrites.
-// ---------------------------------------------------------------------
-
-/// A pre-migration ref's tip reads back through every read surface —
-/// list, show — mapping to state `open` with no context or parent.
-// @relation(meta-ref.migration, scope=function, role=Verifies)
-#[rstest]
-fn phase7_comment_refs_still_read_back() {
- let fixture = Fixture::new();
- fixture.seed_phase7("legacy1", "written by phase-7 code");
-
- let listed = comment::list(&fixture.refs, &fixture.objects).expect("lists");
- assert_eq!(listed.len(), 1);
- let (id, read) = &listed[0];
- assert_eq!(id, "legacy1");
- assert_eq!(read.body, "written by phase-7 code");
- assert_eq!(read.state, "open");
- assert_eq!(read.context, None);
- assert_eq!(read.parent, None);
-
- let (shown, projected) = comment::show(
- &fixture.refs,
- &fixture.objects,
- fixture.path(),
- "legacy1",
- "HEAD",
- false,
- )
- .expect("shows");
- assert_eq!(shown.body, "written by phase-7 code");
- let (anchor, projection) = projected.expect("legacy comments always carry an anchor");
- assert_eq!(anchor.path, "file.txt");
- assert_eq!(projection, ents_anchor::Projection::Current);
-}
-
-/// Mutating a pre-migration ref rewrites its tree under the current
-/// struct as a commit on top of the old tip (`meta-ref.migration`):
-/// the new tip deserializes directly as the broadened `Comment`, its
-/// parent is the untouched legacy commit, and nothing rewrote history.
-// @relation(meta-ref.migration, model.comment-state, scope=function, role=Verifies)
-#[rstest]
-fn mutating_a_phase7_ref_migrates_it_on_top_of_the_old_tip() {
- use gix_object::Find as _;
-
- let fixture = Fixture::new();
- let old_tip = fixture.seed_phase7("legacy1", "written by phase-7 code");
-
- let outcome = comment::resolve(
- &fixture.refs,
- &fixture.objects,
- &NullEventSink,
- "legacy1",
- &fixture.identity(),
- Mode::Advisory,
- )
- .expect("resolves");
- assert_eq!(outcome.result, TxResult::Applied);
-
- let name = ents_model::namespace::comment_ref("legacy1").expect("valid");
- let new_tip = fixture
- .refs
- .get(name.as_ref())
- .expect("readable")
- .expect("set");
- assert_ne!(new_tip, old_tip);
-
- // The new tip's tree is the *current* encoding, no fallback needed...
- let mut buf = Vec::new();
- let data = fixture
- .objects
- .try_find(&new_tip, &mut buf)
- .expect("readable")
- .expect("present");
- let commit = gix_object::CommitRef::from_bytes(data.data, new_tip.kind()).expect("parses");
- let migrated: ents_forge::comment::Comment =
- facet_git_tree::deserialize(&commit.tree(), &fixture.objects)
- .expect("the migrated tip deserializes directly as the broadened struct");
- assert_eq!(migrated.state, "resolved");
- assert_eq!(migrated.body, "written by phase-7 code");
- assert!(migrated.anchor.is_some());
-
- // ...and it sits on top of the old tip: history keeps the old
- // encoding as archive, nothing was rewritten or deleted.
- let parents: Vec<_> = commit.parents().collect();
- assert_eq!(parents, vec![old_tip]);
-}
-
-/// The legacy fallback holds for any body content, not just the fixture
-/// string — old-shape trees over an unenumerable input space read back
-/// with the same mapping.
-// @relation(meta-ref.migration, scope=function, role=Verifies)
-#[test]
-fn any_phase7_tree_reads_back_mapped_to_open() {
- let runner = proptest::test_runner::Config::with_cases(64);
- proptest::proptest!(runner, |(body in proptest::prelude::any::<String>())| {
- let objects = ObjectStore::default();
- let refs = MemRefStore::default();
- let anchor_tree = objects
- .write(&gix_object::Tree { entries: vec![] })
- .expect("tree");
- let legacy = Phase7Comment {
- body: body.clone(),
- anchor: RawTree::new(anchor_tree),
- };
- let name = ents_model::namespace::comment_ref("p").expect("valid");
- write_meta_entity(&refs, &objects, name, &legacy, None, 500);
-
- let listed = comment::list(&refs, &objects).expect("lists");
- proptest::prop_assert_eq!(listed.len(), 1);
- let read = &listed[0].1;
- proptest::prop_assert_eq!(&read.body, &body);
- proptest::prop_assert_eq!(&read.state, "open");
- proptest::prop_assert_eq!(read.anchor.as_ref(), Some(&RawTree::new(anchor_tree)));
- proptest::prop_assert_eq!(&read.context, &None);
- proptest::prop_assert_eq!(&read.parent, &None);
- });
}
// ---------------------------------------------------------------------
crates/forge/ents-forge/src/comment/command.rs
@@ -47,9 +47,8 @@
Ok(commit.tree())
}
-/// Read the [`Comment`] at `id`'s ref tip, through the legacy-shape
-/// fallback (`meta-ref.migration`), or [`Error::NotFound`] when no such
-/// ref exists.
+/// Read the [`Comment`] at `id`'s ref tip, or [`Error::NotFound`] when no
+/// such ref exists.
fn comment_at(refs: &dyn RefStoreRead, objects: &impl Find, id: &str) -> Result<Comment> {
let ref_name = ents_model::namespace::comment_ref(id)?;
let Some(tip) = refs.get(ref_name.as_ref())? else {
@@ -61,8 +60,7 @@
read_comment(&tree, objects)
}
-/// `git ents comment list`: every comment recorded in this repository,
-/// pre-migration trees included (`meta-ref.migration`).
+/// `git ents comment list`: every comment recorded in this repository.
///
/// # Errors
///
@@ -374,10 +372,8 @@
}
/// The shared state mutation [`resolve`] and [`reopen`] are: read the
-/// comment at `id` — through the legacy fallback, so a pre-migration ref
-/// is rewritten under the broadened struct by this very commit
-/// (`meta-ref.migration`) — set `state`, and propose the new tree on top
-/// of the old tip.
+/// comment at `id`, set `state`, and propose the new tree on top of the
+/// old tip.
fn set_state(
refs: &dyn RefStore,
objects: &(impl Find + Write),
crates/forge/ents-forge/src/comment/entity.rs
@@ -2,7 +2,7 @@
//! entity, a parent comment, or any combination.
//!
//! Spec coverage: `model.comment`, `model.comment-state`,
-//! `model.comment-context`, `model.comment-thread`, `meta-ref.migration`.
+//! `model.comment-context`, `model.comment-thread`.
use facet::Facet;
use facet_git_tree::RawTree;
@@ -28,12 +28,6 @@
/// [`ents_anchor`], this crate's own dependency for anchoring a comment to
/// code (`super::command`).
///
-/// Comment trees written before this struct broadened (a bare
-/// `{body, anchor}` shape) still read back through
-/// [`read_comment`]'s legacy fallback (`meta-ref.migration`); mutating one
-/// rewrites its tree under this struct as an ordinary commit on top of the
-/// old tip, keeping the old encoding as archive.
-///
/// # Examples
///
/// ```
@@ -94,61 +88,13 @@
}
}
-/// The comment tree shape phase-7 code wrote: a body and a mandatory,
-/// directly-embedded anchor — no state, context, or parent. Kept only as
-/// [`read_comment`]'s fallback target (`meta-ref.migration`: history keeps
-/// the old encoding as archive, and the tip of a pre-migration ref *is*
-/// still this encoding until something mutates it).
-#[derive(Debug, Clone, PartialEq, Eq, Facet)]
-pub(crate) struct LegacyComment {
- pub(crate) body: String,
- pub(crate) anchor: RawTree,
-}
-
-impl From<LegacyComment> for Comment {
- fn from(legacy: LegacyComment) -> Self {
- Self {
- body: legacy.body,
- state: "open".to_owned(),
- anchor: Some(legacy.anchor),
- context: None,
- parent: None,
- }
- }
-}
-
-/// Read the [`Comment`] stored at `tree`, falling back to the legacy
-/// `{body, anchor}` shape (`meta-ref.migration`).
-///
-/// The two encodings are structurally disjoint, so no version marker is
-/// consulted (`meta-ref.typed-tree` forbids one in the tree, and the
-/// reserved `Schema-Version:` trailer stays unused while detection works
-/// structurally): a legacy tree has no `state` entry and embeds its anchor
-/// tree directly where the broadened struct expects an `Option` wrapper,
-/// so it can never be misread as a current [`Comment`] — and a current tree
-/// always carries `state`, so it is never consulted against the legacy
-/// shape at all.
-///
-/// A legacy read maps to the broadened struct exactly as the migration
-/// commit would rewrite it: state `open` (`model.comment-state`'s value
-/// for every comment created before states existed), no context, no
-/// parent.
+/// Read the [`Comment`] stored at `tree`.
///
/// # Errors
///
-/// The current shape's own [`facet_git_tree::Error`] when `tree` reads as
-/// neither encoding.
-// @relation(meta-ref.migration, scope=function)
+/// A [`facet_git_tree::Error`] when `tree` is not a well-formed comment.
pub(crate) fn read_comment(tree: &ObjectId, objects: &impl Find) -> crate::Result<Comment> {
- match facet_git_tree::deserialize::<Comment>(tree, objects) {
- Ok(comment) => Ok(comment),
- Err(error) => match facet_git_tree::deserialize::<LegacyComment>(tree, objects) {
- Ok(legacy) => Ok(legacy.into()),
- // Report the *current* shape's failure: a tree that is neither
- // encoding is diagnosed against the schema in force.
- Err(_legacy_error) => Err(error.into()),
- },
- }
+ Ok(facet_git_tree::deserialize::<Comment>(tree, objects)?)
}
#[cfg(test)]
@@ -190,32 +136,10 @@
assert_eq!(back, comment);
}
- /// `meta-ref.migration`: a tree written by phase-7 code — the bare
- /// `{body, anchor}` shape — still reads back, mapping to state `open`
- /// with no context or parent; a current tree reads as itself.
- // @relation(meta-ref.migration, scope=function, role=Verifies)
+ /// A tree that is not a well-formed comment fails to read.
+ // @relation(model.comment, scope=function, role=Verifies)
#[rstest]
- fn legacy_trees_still_read_back() {
- let store = ObjectStore::default();
- let legacy = LegacyComment {
- body: "written by phase-7 code".to_owned(),
- anchor: anchor_tree(&store),
- };
- let root = serialize_into(&legacy, &store).expect("serialize");
-
- let read = read_comment(&root, &store).expect("legacy fallback reads");
- assert_eq!(read.body, legacy.body);
- assert_eq!(read.state, "open");
- assert_eq!(read.anchor, Some(legacy.anchor));
- assert_eq!(read.context, None);
- assert_eq!(read.parent, None);
- }
-
- /// A tree that is neither encoding fails against the schema in force,
- /// not the archival one.
- // @relation(meta-ref.migration, scope=function, role=Verifies)
- #[rstest]
- fn a_foreign_tree_reads_as_neither_encoding() {
+ fn a_foreign_tree_fails_to_read() {
let store = ObjectStore::default();
let root = store
.write(&gix_object::Tree { entries: vec![] })