Canonicalize edit operations at ingress

src/apply.rs

@@ -390,7 +390,7 @@ fn changeset_has_move(changeset: &FileChange) -> bool {
     changeset
         .operations
         .iter()
-        .any(|operation| matches!(operation.op, OpKind::Move { .. }))
+        .any(|operation| matches!(operation.op(), OpKind::Move { .. }))
 }
 
 #[cfg(test)]

src/apply/move_ops.rs

@@ -5,7 +5,7 @@ use std::fs;
 use std::io;
 use std::path::{Component, Path, PathBuf};
 
-use crate::changeset::{ChangeOp, FileChange, OpKind, TransformTarget};
+use crate::changeset::{ChangeOp, FileChange};
 use crate::error::IdenteditError;
 
 use super::io::{
@@ -51,12 +51,12 @@ fn validate_file_move_operation_constraints(
     let move_count = changeset
         .operations
         .iter()
-        .filter(|operation| matches!(operation.op, OpKind::Move { .. }))
+        .filter(|operation| operation.as_file_move().is_some())
         .count();
     let has_content_edit = changeset
         .operations
         .iter()
-        .any(|operation| !matches!(operation.op, OpKind::Move { .. }));
+        .any(|operation| operation.as_file_move().is_none());
 
     if move_count > 1 {
         return Err(IdenteditError::InvalidRequest {
@@ -81,81 +81,48 @@ fn validate_file_move_operation_constraints(
     let move_operation = changeset
         .operations
         .iter()
-        .find(|operation| matches!(operation.op, OpKind::Move { .. }))
+        .find_map(ChangeOp::as_file_move)
         .ok_or_else(|| IdenteditError::InvalidRequest {
             message: format!(
                 "Internal validation error: expected one move operation for '{}'",
                 changeset.file.display()
             ),
         })?;
-    let destination = match &move_operation.op {
-        OpKind::Move { to } => to.clone(),
-        _ => unreachable!("move_operation must be move"),
-    };
-    let expected_file_hash = match &move_operation.target {
-        TransformTarget::File { expected_file_hash } => expected_file_hash.clone(),
-        _ => {
-            return Err(IdenteditError::InvalidRequest {
-                message: format!(
-                    "Move operation requires a 'file' target with expected_file_hash: '{}'",
-                    changeset.file.display()
-                ),
-            });
-        }
-    };
-    validate_move_preview(changeset, move_operation, &destination)?;
+    validate_move_preview(
+        changeset,
+        move_operation.preview,
+        move_operation.destination,
+    )?;
 
     Ok(Some(MoveEdge {
         source: changeset.file.clone(),
-        destination,
-        expected_file_hash,
+        destination: move_operation.destination.to_path_buf(),
+        expected_file_hash: move_operation.expected_file_hash.to_string(),
     }))
 }
 
 fn validate_move_preview(
     changeset: &FileChange,
-    operation: &ChangeOp,
+    preview: &crate::changeset::MoveChangePreview,
     destination: &Path,
 ) -> Result<(), IdenteditError> {
-    match &operation.preview {
-        crate::changeset::ChangePreview::Move(preview) => {
-            let Some(move_preview) = preview.move_preview.as_ref() else {
-                // Backward-compatible path: move preview payload may be omitted.
-                return Ok(());
-            };
-
-            if move_preview.from != changeset.file || move_preview.to != destination {
-                return Err(IdenteditError::InvalidRequest {
-                    message: format!(
-                        "Move preview mismatch for '{}': expected move.from='{}' and move.to='{}'",
-                        changeset.file.display(),
-                        changeset.file.display(),
-                        destination.display(),
-                    ),
-                });
-            }
-
-            Ok(())
-        }
-        crate::changeset::ChangePreview::Text(preview) => {
-            if preview.old_text.as_deref().unwrap_or("").is_empty()
-                && preview.old_hash.is_none()
-                && preview.old_len.is_none()
-                && preview.new_text.is_empty()
-                && preview.matched_span.start == 0
-                && preview.matched_span.end == 0
-            {
-                return Ok(());
-            }
+    let Some(move_preview) = preview.move_preview.as_ref() else {
+        // Backward-compatible payloads are normalized to an absent move preview at ingress.
+        return Ok(());
+    };
 
-            Err(IdenteditError::InvalidRequest {
-                message: format!(
-                    "Move operation for '{}' must use move preview fields or compatibility empty text preview",
-                    changeset.file.display()
-                ),
-            })
-        }
+    if move_preview.from != changeset.file || move_preview.to != destination {
+        return Err(IdenteditError::InvalidRequest {
+            message: format!(
+                "Move preview mismatch for '{}': expected move.from='{}' and move.to='{}'",
+                changeset.file.display(),
+                changeset.file.display(),
+                destination.display(),
+            ),
+        });
     }
+
+    Ok(())
 }
 
 fn validate_move_graph(move_edges: &[MoveEdge]) -> Result<Vec<NormalizedMoveEdge>, IdenteditError> {

src/apply/preflight.rs

@@ -176,7 +176,7 @@ fn preflight_changeset(
         || changeset
             .operations
             .iter()
-            .any(|operation| operation.target.requires_node_resolution());
+            .any(|operation| operation.target().requires_node_resolution());
     let handles = if requires_structure_parse {
         parse_handles_for_source_with_registry(&changeset.file, source_text.as_bytes(), registry)?
     } else {

src/apply/replacements.rs

@@ -128,8 +128,7 @@ pub(super) fn apply_replacements_to_text(
 
 fn text_preview(operation: &ChangeOp, index: usize) -> Result<&TextChangePreview, IdenteditError> {
     operation
-        .preview
-        .as_text()
+        .text_preview()
         .ok_or_else(|| IdenteditError::InvalidRequest {
             message: format!(
                 "Operation {} preview must use text preview fields for this operation family",
@@ -165,7 +164,7 @@ pub(super) fn validate_preview_consistency(
             });
         }
 
-        let op_new_text = match &operation.op {
+        let op_new_text = match operation.op() {
             OpKind::Replace { new_text } => new_text,
             OpKind::Delete => "",
             OpKind::InsertBefore { new_text } => new_text,
@@ -201,13 +200,13 @@ fn validate_target_preview_span_consistency(
     let TransformTarget::Node {
         span_hint: Some(span_hint),
         ..
-    } = &operation.target
+    } = operation.target()
     else {
         return Ok(());
     };
     let preview = text_preview(operation, index)?;
 
-    let expected_preview_span = match operation.op {
+    let expected_preview_span = match operation.op() {
         OpKind::Replace { .. }
         | OpKind::Delete
         | OpKind::MoveBefore { .. }
@@ -236,7 +235,7 @@ fn validate_target_preview_span_consistency(
 
 fn allow_stale_preview_span(operation: &crate::changeset::ChangeOp) -> bool {
     if !matches!(
-        operation.op,
+        operation.op(),
         OpKind::Replace { .. }
             | OpKind::Delete
             | OpKind::MoveBefore { .. }
@@ -248,14 +247,13 @@ fn allow_stale_preview_span(operation: &crate::changeset::ChangeOp) -> bool {
     let TransformTarget::Node {
         span_hint: Some(span_hint),
         ..
-    } = &operation.target
+    } = operation.target()
     else {
         return false;
     };
 
     operation
-        .preview
-        .as_text()
+        .text_preview()
         .is_some_and(|preview| preview.matched_span == *span_hint)
 }
 

src/apply/tests/locking.rs

@@ -4,10 +4,9 @@ use std::time::Duration;
 
 use tempfile::tempdir;
 
-use crate::changeset::{OpKind, TransformTarget};
+use crate::changeset::{EditOperation, OpKind, TransformTarget};
 use crate::error::IdenteditError;
 use crate::hash::hash_text;
-use crate::transform::TransformInstruction;
 use crate::transform::build::{build_changeset, build_delete_changeset, build_replace_changeset};
 use crate::transform::parse::parse_handles_for_file;
 
@@ -195,17 +194,20 @@ fn concurrent_apply_with_delete_and_insert_has_single_winner_and_resource_busy_l
         .expect("delete changeset should be built");
     let insert_changeset = build_changeset(
         &file_path,
-        vec![TransformInstruction {
-            target: TransformTarget::node(
-                process_handle.identity.clone(),
-                process_handle.kind.clone(),
-                Some(process_handle.span),
-                hash_text(&process_handle.text),
-            ),
-            op: OpKind::InsertBefore {
-                new_text: "# inserted-by-loser\n".to_string(),
-            },
-        }],
+        vec![
+            EditOperation::try_new(
+                TransformTarget::node(
+                    process_handle.identity.clone(),
+                    process_handle.kind.clone(),
+                    Some(process_handle.span),
+                    hash_text(&process_handle.text),
+                ),
+                OpKind::InsertBefore {
+                    new_text: "# inserted-by-loser\n".to_string(),
+                },
+            )
+            .expect("insert operation should be valid"),
+        ],
     )
     .expect("insert changeset should be built");
 
@@ -276,17 +278,20 @@ fn concurrent_apply_loser_retry_returns_target_missing_after_winner_commits() {
         .expect("delete changeset should be built");
     let insert_changeset = build_changeset(
         &file_path,
-        vec![TransformInstruction {
-            target: TransformTarget::node(
-                process_handle.identity.clone(),
-                process_handle.kind.clone(),
-                Some(process_handle.span),
-                hash_text(&process_handle.text),
-            ),
-            op: OpKind::InsertBefore {
-                new_text: "# inserted-by-loser\n".to_string(),
-            },
-        }],
+        vec![
+            EditOperation::try_new(
+                TransformTarget::node(
+                    process_handle.identity.clone(),
+                    process_handle.kind.clone(),
+                    Some(process_handle.span),
+                    hash_text(&process_handle.text),
+                ),
+                OpKind::InsertBefore {
+                    new_text: "# inserted-by-loser\n".to_string(),
+                },
+            )
+            .expect("insert operation should be valid"),
+        ],
     )
     .expect("insert changeset should be built");
 

src/apply/tests/preflight.rs

@@ -45,44 +45,19 @@ fn build_move_changeset_with_hash(
 ) -> FileChange {
     FileChange {
         file: source.to_path_buf(),
-        operations: vec![ChangeOp {
-            target: TransformTarget::File { expected_file_hash },
-            op: OpKind::Move {
-                to: destination.to_path_buf(),
-            },
-            preview: ChangePreview::move_operation(Some(crate::changeset::MovePreview {
-                from: source.to_path_buf(),
-                to: destination.to_path_buf(),
-            })),
-        }],
-    }
-}
-
-#[test]
-fn move_validation_rejects_node_target() {
-    let directory = tempdir().expect("tempdir should be created");
-    let source = directory.path().join("source.py");
-    let destination = directory.path().join("destination.py");
-    std::fs::write(&source, "def moved():\n    return 1\n")
-        .expect("source fixture write should succeed");
-    let mut changeset = build_move_changeset(&source, &destination);
-    changeset.operations[0].target = TransformTarget::node(
-        "ignored-identity".to_string(),
-        "file".to_string(),
-        None,
-        "ignored-hash".to_string(),
-    );
-
-    let error = validate_move_operation_constraints(&[changeset])
-        .expect_err("move must reject non-file targets");
-    match error {
-        IdenteditError::InvalidRequest { message } => {
-            assert!(
-                message.contains("Move operation requires a 'file' target"),
-                "unexpected target diagnostic: {message}"
-            );
-        }
-        other => panic!("unexpected error variant: {other}"),
+        operations: vec![
+            ChangeOp::from_parts(
+                TransformTarget::File { expected_file_hash },
+                OpKind::Move {
+                    to: destination.to_path_buf(),
+                },
+                ChangePreview::move_operation(Some(crate::changeset::MovePreview {
+                    from: source.to_path_buf(),
+                    to: destination.to_path_buf(),
+                })),
+            )
+            .expect("move changeset should be canonical"),
+        ],
     }
 }
 
@@ -213,16 +188,18 @@ fn preflight_fails_fast_and_preserves_all_file_contents() {
         "def process_data(value):\n    return value * 11".to_string(),
     )
     .expect("changeset_b should be built");
-    let stale_span_hint = match &changeset_b.operations[0].target {
+    let stale_span_hint = match changeset_b.operations[0].target() {
         TransformTarget::Node { span_hint, .. } => *span_hint,
         _ => None,
     };
-    changeset_b.operations[0].target = TransformTarget::node(
-        "missing-preflight-identity".to_string(),
-        "function_definition".to_string(),
-        stale_span_hint,
-        "stale-hash".to_string(),
-    );
+    changeset_b.operations[0]
+        .replace_target(TransformTarget::node(
+            "missing-preflight-identity".to_string(),
+            "function_definition".to_string(),
+            stale_span_hint,
+            "stale-hash".to_string(),
+        ))
+        .expect("node replacement target should remain canonical");
 
     let registry = ProviderRegistry::default();
     let error = preflight_changesets_in_order(&[changeset_a, changeset_b], &registry)
@@ -1845,7 +1822,9 @@ fn move_validation_allows_missing_move_preview_payload_for_backward_compatibilit
     std::fs::write(&source, "def move_me():\n    return 1\n").expect("fixture write should work");
 
     let mut move_changeset = build_move_changeset(&source, &destination);
-    move_changeset.operations[0].preview = ChangePreview::move_operation(None);
+    move_changeset.operations[0]
+        .replace_preview(ChangePreview::move_operation(None))
+        .expect("missing move preview payload should remain canonical");
 
     let plans = validate_move_operation_constraints(&[move_changeset])
         .expect("missing move preview should remain backward-compatible");
@@ -1860,11 +1839,14 @@ fn move_validation_rejects_mismatched_move_preview_paths() {
     std::fs::write(&source, "def move_me():\n    return 1\n").expect("fixture write should work");
 
     let mut move_changeset = build_move_changeset(&source, &destination);
-    move_changeset.operations[0].preview =
-        ChangePreview::move_operation(Some(crate::changeset::MovePreview {
-            from: directory.path().join("other.py"),
-            to: destination.clone(),
-        }));
+    move_changeset.operations[0]
+        .replace_preview(ChangePreview::move_operation(Some(
+            crate::changeset::MovePreview {
+                from: directory.path().join("other.py"),
+                to: destination.clone(),
+            },
+        )))
+        .expect("mismatched move paths still use the canonical move preview family");
 
     let error = validate_move_operation_constraints(&[move_changeset])
         .expect_err("mismatched move preview should be rejected");