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

[aaltshuler]

Summary

Wires up DropType { Soft } end-to-end — the second half of the dormant Drop* family from chassis v1.0 (PR #90). Operators can now remove a node or edge type from a .pg schema and omnigraph schema apply will succeed, tombstoning the __manifest entry while retaining Lance dataset files for time-travel reversibility (until omnigraph cleanup runs).

Builds directly on commit #3 (chassis v1.0). Doesn't include Hard mode + --allow-data-loss (commit #5 — separate PR).

What lands

Planner (schema_plan.rs):

• plan_nodes leftover loop: emits DropType { Node, name, Soft } instead of UnsupportedChange (OG-DS-102).
• plan_edges leftover loop: emits DropType { Edge, name, Soft } instead of UnsupportedChange (OG-DS-103).

Apply (schema_apply.rs):

• New dropped_tables: BTreeSet<String> accumulator alongside added_tables / renamed_tables / rewritten_tables.
• DropType arm in the metadata loop populates dropped_tables for Soft mode. Hard mode errors (commit Add schema apply command and policy support #5).
• New tombstone-emission loop after the rename sidecar build: each dropped table → sidecar_tombstones AND table_tombstones with table_version + 1. The existing manifest publish path converts table_tombstones into ManifestChange::Tombstone operations — no new manifest plumbing needed.
• Soft DropType has no Phase B per-table write. The tombstone is the entire change. Lance dataset files are retained because prior __manifest versions still reference them.
• Recovery: rides on SidecarKind::SchemaApply per MR-847 (already established).

Tests:

• Planner unit test: plan_emits_soft_drop_for_removed_node_and_edge_types (asserts both Node + Edge variant emission for the Company + WorksAt combined drop).
• Integration test: apply_schema_drops_node_and_referencing_edge_softly (replaces apply_schema_rejects_dropping_a_node_type). Asserts plan emission, apply success, current manifest entries absent, pre-drop manifest entries present, reopen consistency.
• Integration test: apply_schema_drops_an_edge_type_softly (replaces apply_schema_rejects_dropping_an_edge_type). Single edge drop, asserts other tables untouched, time-travel reversibility.

What's deliberately not in this PR

• --allow-data-loss flag + Hard mode — commit Add schema apply command and policy support #5 (separate PR). Adds CLI flag, planner Soft→Hard promotion when set, apply path runs compact_files + cleanup_old_versions for both DropProperty Hard and DropType Hard.
• Lance native column ops via LanceColumnOp — MR-948 (substrate-alignment refactor). v1.1 doesn't introduce this surface; commit Add support for null literals in query parameters #3's stage_overwrite path for DropProperty is unchanged.

Test plan

• cargo test -p omnigraph-compiler --lib — 239 pass (1 new + 238 existing)
• cargo test -p omnigraph-engine --test schema_apply — 11 pass (2 converted + 9 unchanged)
• No regressions in: AddProperty, RenameProperty, RenameType, AddConstraint, UpdateTypeMetadata, UpdatePropertyMetadata
• Pre-drop snapshot_at_version(N) still resolves dropped tables (verified by integration test)
• Reopen consistency: drop preserved across engine restart

Follow-up

• Commit Add schema apply command and policy support #5: Hard mode + --allow-data-loss flag (after this merges)
• MR-700 ticket: this PR completes the Soft half of DropType + DropProperty. Hard mode + flag are the only pending pieces.

🤖 Generated with Claude Code

---

[cubic-dev-ai]

cubic analysis

2 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/omnigraph-compiler/src/catalog/schema_plan.rs">

<violation number="1" location="crates/omnigraph-compiler/src/catalog/schema_plan.rs:335">
P2: According to linked Linear issue MR-694, migration diagnostics should stay code-tagged for rule-level severity/suppression. Emitting `DropType` here without any code mapping drops OG-DS-102/103 from planner output, so drop-type rules can no longer be configured/suppressed via the chassis.</violation>
</file>

<file name="crates/omnigraph/tests/schema_apply.rs">

<violation number="1" location="crates/omnigraph/tests/schema_apply.rs:301">
P2: This test claims both Person and Knows remain, but it only asserts Person. Add an assertion for `edge:Knows` so regressions that drop unrelated edges are caught.</violation>
</file>

Linked issue analysis

Linked issue: MR-694: Schema-lint chassis: classification tuple + per-rule severity + suppression + pre-migration checks

Status	Acceptance criteria	Notes
✅	Planner emits DropType { type_kind, name, DropMode::Soft } for removed node and edge types	schema_plan.rs now pushes SchemaMigrationStep::DropType with type_kind/name/mode and a unit test (plan_emits_soft_drop_for_removed_node_and_edge_types) was added that asserts both Node and Edge Soft drops appear in the plan.
✅	Apply path honors DropType Soft by marking tables for tombstoning (no Phase B per-table write) and rejects non-Soft (Hard) mode	schema_apply now handles SchemaMigrationStep::DropType { .. }, inserts table keys into dropped_tables, rejects non-Soft modes, and later emits sidecar_tombstones and table_tombstones; comments and code indicate no Phase B per-table write for Soft drops.
✅	Integration and unit tests validate plan/apply behavior, time-travel reversibility, and reopen consistency for Soft DropType	Tests were converted from expecting rejections to asserting plan.supported, presence of DropType steps, successful apply, manifest version advance, current snapshot omits dropped tables, pre-drop snapshot still lists them, and reopen preserves the drop.
⚠️	Recovery sidecar registration for Soft drops (manifest-sidecar tombstones emitted)	The PR emits SidecarTombstone entries and records table_tombstones for the manifest publish path, which implements the recovery-sidecar discipline. The diff shows sidecar_tombstones population, but an explicit SidecarKind::SchemaApply registration call is not visible in the provided changes (the PR message claims recovery rides on SidecarKind::SchemaApply per MR-847).

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic}

[cubic-dev-ai]

P2: According to linked Linear issue MR-694, migration diagnostics should stay code-tagged for rule-level severity/suppression. Emitting DropType here without any code mapping drops OG-DS-102/103 from planner output, so drop-type rules can no longer be configured/suppressed via the chassis.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At crates/omnigraph-compiler/src/catalog/schema_plan.rs, line 335:

<comment>According to linked Linear issue MR-694, migration diagnostics should stay code-tagged for rule-level severity/suppression. Emitting `DropType` here without any code mapping drops OG-DS-102/103 from planner output, so drop-type rules can no longer be configured/suppressed via the chassis.</comment>

<file context>
@@ -324,13 +324,18 @@ fn plan_nodes(
+        // 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(),
</file context>

[cubic-dev-ai]

P2: This test claims both Person and Knows remain, but it only asserts Person. Add an assertion for edge:Knows so regressions that drop unrelated edges are caught.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At crates/omnigraph/tests/schema_apply.rs, line 301:

<comment>This test claims both Person and Knows remain, but it only asserts Person. Add an assertion for `edge:Knows` so regressions that drop unrelated edges are caught.</comment>

<file context>
@@ -241,23 +245,96 @@ edge Knows: Person -> Person {
+    );
+    // 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",
+    );
</file context>

Suggested change

	current_snapshot.entry("node:Person").is_some(),
	"node:Person must remain in the manifest",
	);
	assert!(
	current_snapshot.entry("node:Person").is_some(),
	"node:Person must remain in the manifest",
	);
	assert!(
	current_snapshot.entry("edge:Knows").is_some(),
	"edge:Knows must remain in the manifest",
	);

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.