schema-lint chassis v1.1: DropType Soft (MR-694)

crates/omnigraph-compiler/src/catalog/schema_plan.rs

@@ -324,13 +324,18 @@ fn plan_nodes(
         .iter()
         .filter(|node| !consumed.contains(&node.name))
     {
-        steps.push(SchemaMigrationStep::UnsupportedChange {
-            entity: format!("node:{}", leftover.name),
-            reason: format!(
-                "removing node type '{}' is not supported in schema migration v1",
-                leftover.name
-            ),
-            code: Some(crate::lint::codes::OG_DS_102.code.to_string()),
+        // Node type removed from the desired schema: emit
+        // DropType { Node, Soft } per docs/dev/schema-lint-v1-plan.md
+        // commit #4. Soft = remove the table's entry from the current
+        // __manifest version; data files retained; previous manifest
+        // versions still reference the table, so Lance time travel
+        // restores it until cleanup_old_versions ages out the older
+        // __manifest entries. Hard mode (immediate dataset deletion)
+        // lands in commit #5 gated by --allow-data-loss.
+        steps.push(SchemaMigrationStep::DropType {
+            type_kind: SchemaTypeKind::Node,
+            name: leftover.name.clone(),
+            mode: DropMode::Soft,
         });
     }
 
@@ -442,13 +447,15 @@ fn plan_edges(
         .iter()
         .filter(|edge| !consumed.contains(&edge.name))
     {
-        steps.push(SchemaMigrationStep::UnsupportedChange {
-            entity: format!("edge:{}", leftover.name),
-            reason: format!(
-                "removing edge type '{}' is not supported in schema migration v1",
-                leftover.name
-            ),
-            code: Some(crate::lint::codes::OG_DS_103.code.to_string()),
+        // Edge type removed from the desired schema: emit
+        // DropType { Edge, Soft } per docs/dev/schema-lint-v1-plan.md
+        // commit #4. Same Soft mechanics as node-type drops — manifest
+        // entry tombstoned, data files retained, reversible via Lance
+        // time travel until cleanup.
+        steps.push(SchemaMigrationStep::DropType {
+            type_kind: SchemaTypeKind::Edge,
+            name: leftover.name.clone(),
+            mode: DropMode::Soft,
         });
     }
 }
@@ -991,6 +998,81 @@ node Person {
         );
     }
 
+    #[test]
+    fn plan_emits_soft_drop_for_removed_node_and_edge_types() {
+        // Removing a node type + the edge type that references it
+        // emits two DropType { Soft } steps (chassis v1 commit #4,
+        // MR-694). The plan is `supported = true` — apply tombstones
+        // both manifest entries. Time-travel reversibility is verified
+        // at the integration level by
+        // `apply_schema_drops_node_and_referencing_edge_softly`
+        // in `crates/omnigraph/tests/schema_apply.rs`.
+        let accepted = build_schema_ir(
+            &parse_schema(
+                r#"
+node Person {
+    name: String @key
+}
+
+node Company {
+    name: String @key
+}
+
+edge WorksAt: Person -> Company
+"#,
+            )
+            .unwrap(),
+        )
+        .unwrap();
+        let desired = build_schema_ir(
+            &parse_schema(
+                r#"
+node Person {
+    name: String @key
+}
+"#,
+            )
+            .unwrap(),
+        )
+        .unwrap();
+
+        let plan = plan_schema_migration(&accepted, &desired).unwrap();
+        assert!(
+            plan.supported,
+            "drop-type plan must be supported: {plan:?}"
+        );
+        assert!(
+            plan.steps.iter().any(|step| matches!(
+                step,
+                SchemaMigrationStep::DropType {
+                    type_kind: SchemaTypeKind::Node,
+                    name,
+                    mode: DropMode::Soft,
+                } if name == "Company"
+            )),
+            "expected DropType {{ Node, Company, Soft }} in plan: {plan:?}",
+        );
+        assert!(
+            plan.steps.iter().any(|step| matches!(
+                step,
+                SchemaMigrationStep::DropType {
+                    type_kind: SchemaTypeKind::Edge,
+                    name,
+                    mode: DropMode::Soft,
+                } if name == "WorksAt"
+            )),
+            "expected DropType {{ Edge, WorksAt, Soft }} in plan: {plan:?}",
+        );
+        // Negative: no UnsupportedChange anywhere in the plan.
+        assert!(
+            !plan
+                .steps
+                .iter()
+                .any(|step| matches!(step, UnsupportedChange { .. })),
+            "soft type drop must not emit UnsupportedChange: {plan:?}",
+        );
+    }
+
     #[test]
     fn plan_rejects_required_property_addition() {
         let accepted = build_schema_ir(

crates/omnigraph/src/db/omnigraph/schema_apply.rs

@@ -78,6 +78,7 @@ pub(super) async fn apply_schema_with_lock(
     let mut renamed_tables = HashMap::new();
     let mut rewritten_tables = BTreeSet::new();
     let mut indexed_tables = BTreeSet::new();
+    let mut dropped_tables = BTreeSet::new();
     let mut property_renames = HashMap::<String, HashMap<String, String>>::new();
     let mut changed_edge_tables = false;
 
@@ -165,13 +166,31 @@ pub(super) async fn apply_schema_with_lock(
                 }
                 rewritten_tables.insert(table_key);
             }
-            SchemaMigrationStep::DropType { .. } => {
-                // DropType (whole-table drop via __manifest entry
-                // removal) lands in commit #4 — different mechanics
-                // from DropProperty.
-                return Err(OmniError::manifest_internal(
-                    "DropType not yet implemented (commit #4)",
-                ));
+            SchemaMigrationStep::DropType {
+                type_kind,
+                name,
+                mode,
+            } => {
+                // Soft = remove the table's entry from the current
+                // __manifest version via a tombstone. The Lance
+                // dataset files are retained — prior __manifest
+                // versions still reference them, so Lance time
+                // travel + branch-from-snapshot can read the dropped
+                // table until `omnigraph cleanup` ages out the older
+                // manifest versions. No per-table write happens here;
+                // the tombstone is the entire change. Hard mode
+                // (immediate dataset deletion via cleanup) lands in
+                // commit #5 gated by --allow-data-loss.
+                if !matches!(mode, DropMode::Soft) {
+                    return Err(OmniError::manifest_internal(
+                        "DropType { Hard } not yet implemented (commit #5)",
+                    ));
+                }
+                let table_key = schema_table_key(*type_kind, name);
+                if table_key.starts_with("edge:") {
+                    changed_edge_tables = true;
+                }
+                dropped_tables.insert(table_key);
             }
             step @ SchemaMigrationStep::UnsupportedChange { .. } => {
                 return Err(OmniError::manifest(
@@ -243,6 +262,26 @@ pub(super) async fn apply_schema_with_lock(
             tombstone_version: source_entry.table_version.saturating_add(1),
         });
     }
+    // Soft DropType: mark each dropped table for tombstoning in the
+    // recovery sidecar AND in the live table_tombstones map. The
+    // mechanism mirrors rename's source-table tombstone — manifest
+    // entry removed at version+1, dataset files retained, time-travel
+    // reachable until cleanup. No Phase B write happens for these
+    // tables; the recovery sidecar is purely the manifest delta.
+    for dropped_table_key in &dropped_tables {
+        let entry = snapshot.entry(dropped_table_key).ok_or_else(|| {
+            OmniError::manifest(format!(
+                "missing table '{}' for soft drop when building recovery sidecar",
+                dropped_table_key
+            ))
+        })?;
+        let tombstone_version = entry.table_version.saturating_add(1);
+        sidecar_tombstones.push(crate::db::manifest::SidecarTombstone {
+            table_key: dropped_table_key.clone(),
+            tombstone_version,
+        });
+        table_tombstones.insert(dropped_table_key.clone(), tombstone_version);
+    }
 
     // Acquire per-(table_key, branch) queues for every existing table
     // that schema_apply will rewrite or re-index. New tables (added or

crates/omnigraph/tests/schema_apply.rs

@@ -221,7 +221,7 @@ async fn apply_schema_drops_a_nullable_property_softly_preserves_prior_version()
 }
 
 #[tokio::test]
-async fn apply_schema_rejects_dropping_a_node_type() {
+async fn apply_schema_drops_node_and_referencing_edge_softly() {
     let dir = tempfile::tempdir().unwrap();
     let mut db = init_and_load(&dir).await;
     let before_version = db
@@ -230,7 +230,11 @@ async fn apply_schema_rejects_dropping_a_node_type() {
         .unwrap()
         .version();
 
-    // Drop the `Company` node type and its outgoing edge that references it.
+    // Drop the `Company` node type and the `WorksAt` edge that references it.
+    // Per schema-lint v1 chassis commit #4 (MR-694), this emits two
+    // `DropType { Soft }` steps; apply tombstones both manifest entries.
+    // Lance dataset files are retained, so time-travel back to the
+    // pre-drop manifest version still resolves both tables.
     let desired = r#"
 node Person {
     name: String @key
@@ -241,23 +245,96 @@ edge Knows: Person -> Person {
     since: Date?
 }
 "#;
-    let err = db.apply_schema(desired).await.unwrap_err();
-    let msg = err.to_string();
+
+    // Confirm the plan emits both DropType { Soft } steps.
+    let plan = db.plan_schema(desired).await.unwrap();
+    assert!(plan.supported, "drop-type plan must be supported");
     assert!(
-        msg.contains("OG-DS-102") || msg.contains("OG-DS-103"),
-        "expected schema-lint code OG-DS-102 or OG-DS-103 in error, got: {msg}"
+        plan.steps.iter().any(|step| matches!(
+            step,
+            SchemaMigrationStep::DropType {
+                type_kind: SchemaTypeKind::Node,
+                name,
+                mode: omnigraph_compiler::DropMode::Soft,
+            } if name == "Company"
+        )),
+        "expected DropType {{ Node, Company, Soft }} in plan: {plan:?}",
     );
-    assert_eq!(
-        db.snapshot_of(ReadTarget::branch("main"))
-            .await
-            .unwrap()
-            .version(),
-        before_version
+    assert!(
+        plan.steps.iter().any(|step| matches!(
+            step,
+            SchemaMigrationStep::DropType {
+                type_kind: SchemaTypeKind::Edge,
+                name,
+                mode: omnigraph_compiler::DropMode::Soft,
+            } if name == "WorksAt"
+        )),
+        "expected DropType {{ Edge, WorksAt, Soft }} in plan: {plan:?}",
+    );
+
+    let result = db.apply_schema(desired).await.unwrap();
+    assert!(result.supported);
+    assert!(result.applied);
+
+    let after_version = db
+        .snapshot_of(ReadTarget::branch("main"))
+        .await
+        .unwrap()
+        .version();
+    assert!(
+        after_version > before_version,
+        "manifest version should advance after soft type drop; before={before_version}, after={after_version}",
+    );
+
+    // (a) Current snapshot: both manifest entries are gone.
+    let current_snapshot = db.snapshot_of(ReadTarget::branch("main")).await.unwrap();
+    assert!(
+        current_snapshot.entry("node:Company").is_none(),
+        "current manifest must not list node:Company after soft drop",
+    );
+    assert!(
+        current_snapshot.entry("edge:WorksAt").is_none(),
+        "current manifest must not list edge:WorksAt after soft drop",
+    );
+    // Person + Knows still present (Person wasn't dropped; Knows is in desired).
+    assert!(
+        current_snapshot.entry("node:Person").is_some(),
+        "node:Person must remain in the manifest",
+    );
+
+    // (b) Time travel: at the pre-drop manifest version, both dropped
+    // tables are still listed. Soft drop is reversible via Lance's
+    // version graph until `omnigraph cleanup` runs.
+    let pre_drop_snapshot = db.snapshot_at_version(before_version).await.unwrap();
+    assert!(
+        pre_drop_snapshot.entry("node:Company").is_some(),
+        "pre-drop manifest must still list node:Company (time-travel reversibility)",
+    );
+    assert!(
+        pre_drop_snapshot.entry("edge:WorksAt").is_some(),
+        "pre-drop manifest must still list edge:WorksAt (time-travel reversibility)",
+    );
+
+    // (c) Reopen consistency: drop is preserved across engine restart.
+    let uri = dir.path().to_str().unwrap().to_string();
+    drop(db);
+    let reopened = Omnigraph::open(&uri).await.unwrap();
+    let reopened_snapshot = reopened
+        .snapshot_of(ReadTarget::branch("main"))
+        .await
+        .unwrap();
+    assert!(
+        reopened_snapshot.entry("node:Company").is_none(),
+        "after reopen, node:Company must still be absent from the current manifest",
+    );
+    assert!(
+        reopened_snapshot.entry("edge:WorksAt").is_none(),
+        "after reopen, edge:WorksAt must still be absent from the current manifest",
     );
 }
 
 #[tokio::test]
-async fn apply_schema_rejects_dropping_an_edge_type() {
+async fn apply_schema_drops_an_edge_type_softly() {
     let dir = tempfile::tempdir().unwrap();
     let mut db = init_and_load(&dir).await;
     let before_version = db
@@ -266,20 +343,50 @@ async fn apply_schema_rejects_dropping_an_edge_type() {
         .unwrap()
         .version();
 
-    // Drop only the `WorksAt` edge.
+    // Drop only the `WorksAt` edge. Per chassis v1 commit #4, this
+    // emits `DropType { Edge, WorksAt, Soft }`; apply tombstones the
+    // edge:WorksAt manifest entry. The Company node and Person node
+    // remain intact.
     let desired = TEST_SCHEMA.replace("\nedge WorksAt: Person -> Company", "");
-    let err = db.apply_schema(&desired).await.unwrap_err();
-    let msg = err.to_string();
+
+    let plan = db.plan_schema(&desired).await.unwrap();
+    assert!(plan.supported);
+    assert!(
+        plan.steps.iter().any(|step| matches!(
+            step,
+            SchemaMigrationStep::DropType {
+                type_kind: SchemaTypeKind::Edge,
+                name,
+                mode: omnigraph_compiler::DropMode::Soft,
+            } if name == "WorksAt"
+        )),
+        "expected DropType {{ Edge, WorksAt, Soft }} in plan: {plan:?}",
+    );
+
+    let result = db.apply_schema(&desired).await.unwrap();
+    assert!(result.applied);
+
+    let after_version = db
+        .snapshot_of(ReadTarget::branch("main"))
+        .await
+        .unwrap()
+        .version();
+    assert!(after_version > before_version);
+
+    let current_snapshot = db.snapshot_of(ReadTarget::branch("main")).await.unwrap();
     assert!(
-        msg.contains("OG-DS-103"),
-        "expected schema-lint code OG-DS-103 in error, got: {msg}"
+        current_snapshot.entry("edge:WorksAt").is_none(),
+        "current manifest must not list edge:WorksAt",
     );
-    assert_eq!(
-        db.snapshot_of(ReadTarget::branch("main"))
-            .await
-            .unwrap()
-            .version(),
-        before_version
+    // Other tables untouched.
+    assert!(current_snapshot.entry("node:Person").is_some());
+    assert!(current_snapshot.entry("node:Company").is_some());
+    assert!(current_snapshot.entry("edge:Knows").is_some());
+
+    let pre_drop_snapshot = db.snapshot_at_version(before_version).await.unwrap();
+    assert!(
+        pre_drop_snapshot.entry("edge:WorksAt").is_some(),
+        "pre-drop manifest must still list edge:WorksAt",
     );
 }