mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-09 01:35:18 +02:00
schema-lint chassis v0: code-tagged diagnostics (MR-694) (#87)
First slice of the schema-lint chassis. Adds stable `OG-XXX-NNN`
codes to schema-migration rejections so operators can suppress, look
up, and filter on identifiers rather than free-text prose. Atlas-style
chassis adapted to omnigraph's typed-IR substrate (no SQL injection
vector, no per-engine locks, native edge/vector/embedding types).
What's in v0:
- New `omnigraph-compiler/src/lint/` module with:
- `diagnostic.rs` — Family / SafetyTier / Severity enums covering ten
families (DS, MF, CD, BC, NM, OW, NL, VE, ED, LK). Only DS and MF
are populated in this PR.
- `codes.rs` — 8 DiagnosticCode constants (OG-DS-101..105,
OG-MF-103, OG-MF-104, OG-MF-106). Five of the eight are wired to
real emission sites; the other three are reserved.
- Unit tests for catalog invariants: codes unique, prefix matches
family, suffixes are 3-digit, destructive defaults to error,
lookup() works, EMITTED_IN_V0 codes exist in ALL_CODES.
- `SchemaMigrationStep::UnsupportedChange` gains an optional
`code: Option<String>` field. New `unsupported_error_message()`
helper prefixes the message with `[code]` when present.
- 5 of 17 existing rejection paths now carry codes:
- `removing node type` → OG-DS-102
- `removing edge type` → OG-DS-103
- `removing property` → OG-DS-104
- `adding required property without backfill` → OG-MF-103
- `changing property type` → OG-MF-106
Remaining 12 paths carry `code: None` and are tagged as future work.
- `schema_apply` surfaces the formatted error (with `[code]` prefix);
CLI `omnigraph schema plan` renders the code on the
`unsupported change on <entity>` line.
- PR #62 destructive-rejection tests in `tests/schema_apply.rs` now
assert on the stable code (`msg.contains("OG-DS-104")`) instead of
the error-message substring. 11/11 tests pass.
- New `docs/schema-lint.md` documents the v0 catalog + the 10 families
+ Atlas prior art. AGENTS.md index updated.
What's explicitly NOT in v0 (subsequent PRs):
- No severity config in `omnigraph.yaml` (MR-694 §2).
- No `@allow(OG-XXX-NNN, "rationale")` suppression directive (§3).
- No `--allow-data-loss` flag or destructive-tier enforcement.
- No new `SchemaMigrationStep` variants (soft/hard drops, default,
widen/narrow). MR-700, MR-697 land those.
- No pre-migration checks (MR-941).
- No CD / VE / LK / NM family rules (MR-942..945).
- No CI integration (MR-946).
Tests: 235 compiler tests, 11 schema_apply integration tests, 14
lint module tests, 55 CLI tests — all green.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f28f644bf2
commit
c142dafdf3
10 changed files with 445 additions and 24 deletions
|
|
@ -73,6 +73,7 @@ Full diagram and concurrency model: [docs/architecture.md](docs/architecture.md)
|
|||
| Architecture, L1/L2 framing, concurrency model | [docs/architecture.md](docs/architecture.md) |
|
||||
| Storage layout, `__manifest` schema, URI schemes, S3 env vars | [docs/storage.md](docs/storage.md) |
|
||||
| `.pg` schema language, types, constraints, annotations, migration planning | [docs/schema-language.md](docs/schema-language.md) |
|
||||
| Schema-lint codes (`OG-XXX-NNN`), families, severity, suppression | [docs/schema-lint.md](docs/schema-lint.md) |
|
||||
| `.gq` query language, MATCH/RETURN/ORDER, search funcs, mutations, IR ops, lint codes | [docs/query-language.md](docs/query-language.md) |
|
||||
| Indexes (BTREE / inverted / vector / graph topology) | [docs/indexes.md](docs/indexes.md) |
|
||||
| Embeddings (compiler + engine clients, env vars, `@embed`) | [docs/embeddings.md](docs/embeddings.md) |
|
||||
|
|
|
|||
|
|
@ -1035,9 +1035,14 @@ fn render_schema_plan_step(step: &SchemaMigrationStep) -> String {
|
|||
type_name,
|
||||
render_annotations(annotations)
|
||||
),
|
||||
SchemaMigrationStep::UnsupportedChange { entity, reason } => {
|
||||
format!("unsupported change on {}: {}", entity, reason)
|
||||
}
|
||||
SchemaMigrationStep::UnsupportedChange {
|
||||
entity,
|
||||
reason,
|
||||
code,
|
||||
} => match code {
|
||||
Some(c) => format!("unsupported change on {} [{}]: {}", entity, c, reason),
|
||||
None => format!("unsupported change on {}: {}", entity, reason),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -65,9 +65,36 @@ pub enum SchemaMigrationStep {
|
|||
UnsupportedChange {
|
||||
entity: String,
|
||||
reason: String,
|
||||
/// Stable schema-lint code (`OG-XXX-NNN`) for this rejection,
|
||||
/// or `None` if the path predates the chassis catalog. See
|
||||
/// [`crate::lint::codes`] for the registry. Renderers should
|
||||
/// prefix the message with `[code]` when present so operators
|
||||
/// can suppress, look up docs, or filter on stable identifiers
|
||||
/// rather than free-text prose.
|
||||
///
|
||||
/// Stored as `String` (not `&'static str`) so the enum stays
|
||||
/// serde-friendly. Emitters pass the catalog constant's
|
||||
/// `.code` (a `&'static str`) and we own a clone here.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
code: Option<String>,
|
||||
},
|
||||
}
|
||||
|
||||
impl SchemaMigrationStep {
|
||||
/// Returns the formatted error message for an `UnsupportedChange`
|
||||
/// step, prefixed with `[code] ` when a schema-lint code is attached.
|
||||
/// Returns `None` for every other variant.
|
||||
pub fn unsupported_error_message(&self) -> Option<String> {
|
||||
match self {
|
||||
Self::UnsupportedChange { reason, code, .. } => Some(match code {
|
||||
Some(c) => format!("[{}] {}", c, reason),
|
||||
None => reason.clone(),
|
||||
}),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn plan_schema_migration(
|
||||
accepted: &SchemaIR,
|
||||
desired: &SchemaIR,
|
||||
|
|
@ -130,6 +157,7 @@ fn plan_interfaces(
|
|||
"removing interface '{}' is not supported in schema migration v1",
|
||||
leftover.name
|
||||
),
|
||||
code: None,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -171,6 +199,7 @@ fn plan_nodes(
|
|||
"node '{}' declares @rename_from(\"{}\") but no accepted node with that name exists",
|
||||
node.name, from
|
||||
),
|
||||
code: None,
|
||||
});
|
||||
} else {
|
||||
steps.push(SchemaMigrationStep::AddType {
|
||||
|
|
@ -200,6 +229,7 @@ fn plan_nodes(
|
|||
"changing implemented interfaces on node '{}' is not supported in schema migration v1",
|
||||
node.name
|
||||
),
|
||||
code: None,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -237,6 +267,7 @@ fn plan_nodes(
|
|||
"removing node type '{}' is not supported in schema migration v1",
|
||||
leftover.name
|
||||
),
|
||||
code: Some(crate::lint::codes::OG_DS_102.code.to_string()),
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -277,6 +308,7 @@ fn plan_edges(
|
|||
"edge '{}' declares @rename_from(\"{}\") but no accepted edge with that name exists",
|
||||
edge.name, from
|
||||
),
|
||||
code: None,
|
||||
});
|
||||
} else {
|
||||
steps.push(SchemaMigrationStep::AddType {
|
||||
|
|
@ -305,6 +337,7 @@ fn plan_edges(
|
|||
"changing edge endpoints on '{}' is not supported in schema migration v1",
|
||||
edge.name
|
||||
),
|
||||
code: None,
|
||||
});
|
||||
}
|
||||
if existing.cardinality != edge.cardinality {
|
||||
|
|
@ -314,6 +347,7 @@ fn plan_edges(
|
|||
"changing cardinality on edge '{}' is not supported in schema migration v1",
|
||||
edge.name
|
||||
),
|
||||
code: None,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -351,6 +385,7 @@ fn plan_edges(
|
|||
"removing edge type '{}' is not supported in schema migration v1",
|
||||
leftover.name
|
||||
),
|
||||
code: Some(crate::lint::codes::OG_DS_103.code.to_string()),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -396,6 +431,7 @@ fn plan_properties(
|
|||
"property '{}.{}' declares @rename_from(\"{}\") but no accepted property with that name exists",
|
||||
type_name, property.name, from
|
||||
),
|
||||
code: None,
|
||||
});
|
||||
} else if property.prop_type.nullable {
|
||||
steps.push(SchemaMigrationStep::AddProperty {
|
||||
|
|
@ -416,6 +452,7 @@ fn plan_properties(
|
|||
"adding required property '{}.{}' requires a backfill and is not supported in schema migration v1",
|
||||
type_name, property.name
|
||||
),
|
||||
code: Some(crate::lint::codes::OG_MF_103.code.to_string()),
|
||||
});
|
||||
}
|
||||
continue;
|
||||
|
|
@ -444,6 +481,7 @@ fn plan_properties(
|
|||
"changing property type for '{}.{}' is not supported in schema migration v1",
|
||||
type_name, property.name
|
||||
),
|
||||
code: Some(crate::lint::codes::OG_MF_106.code.to_string()),
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -472,6 +510,7 @@ fn plan_properties(
|
|||
"removing property '{}.{}' is not supported in schema migration v1",
|
||||
type_name, leftover.name
|
||||
),
|
||||
code: Some(crate::lint::codes::OG_DS_104.code.to_string()),
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -513,6 +552,7 @@ fn plan_constraints(
|
|||
"removing constraints from '{}' is not supported in schema migration v1",
|
||||
type_name
|
||||
),
|
||||
code: None,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -532,6 +572,7 @@ fn plan_constraints(
|
|||
"adding constraint '{}' to '{}' is not supported in schema migration v1",
|
||||
key, type_name
|
||||
),
|
||||
code: None,
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
|
@ -557,6 +598,7 @@ fn plan_type_metadata(
|
|||
steps.push(SchemaMigrationStep::UnsupportedChange {
|
||||
entity: format!("{}:{}", schema_type_kind_key(type_kind), name),
|
||||
reason,
|
||||
code: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -589,6 +631,7 @@ fn plan_property_metadata(
|
|||
property_name
|
||||
),
|
||||
reason,
|
||||
code: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -850,9 +893,9 @@ node Person {
|
|||
assert!(!plan.supported);
|
||||
assert!(plan.steps.iter().any(|step| matches!(
|
||||
step,
|
||||
UnsupportedChange { entity, reason }
|
||||
UnsupportedChange { entity, code, .. }
|
||||
if entity.contains("Person.age")
|
||||
&& reason.contains("adding required property")
|
||||
&& code.as_deref() == Some(crate::lint::codes::OG_MF_103.code)
|
||||
)));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ pub mod embedding;
|
|||
pub mod error;
|
||||
pub mod ir;
|
||||
pub mod json_output;
|
||||
pub mod lint;
|
||||
pub mod query;
|
||||
pub mod query_input;
|
||||
pub mod result;
|
||||
|
|
@ -17,6 +18,7 @@ pub use catalog::schema_ir::{
|
|||
pub use catalog::schema_plan::{
|
||||
SchemaMigrationPlan, SchemaMigrationStep, SchemaTypeKind, plan_schema_migration,
|
||||
};
|
||||
pub use lint::{DiagnosticCode, Family, SafetyTier, Severity};
|
||||
pub use ir::ParamMap;
|
||||
pub use ir::lower::{lower_mutation_query, lower_query};
|
||||
pub use query::ast::Literal;
|
||||
|
|
|
|||
195
crates/omnigraph-compiler/src/lint/codes.rs
Normal file
195
crates/omnigraph-compiler/src/lint/codes.rs
Normal file
|
|
@ -0,0 +1,195 @@
|
|||
//! Schema-lint code catalog (MR-694 v0).
|
||||
//!
|
||||
//! Codes have the form `OG-XXX-NNN` where `XXX` is the family prefix from
|
||||
//! [`super::diagnostic::Family`]. Each code carries a default safety
|
||||
//! tier, severity, and short description. Codes are stable: once
|
||||
//! published, the meaning is frozen and `omnigraph.yaml` may override
|
||||
//! severity but never the tier or family.
|
||||
//!
|
||||
//! ## v0 catalog
|
||||
//!
|
||||
//! This PR (chassis v0) seeds the catalog with the codes attached to
|
||||
//! existing `UnsupportedChange` emissions in
|
||||
//! `catalog::schema_plan`. Subsequent PRs add codes for new migration
|
||||
//! variants (MR-695..702), the CD/VE/LK/NM families, and so on.
|
||||
|
||||
use super::diagnostic::{Family, SafetyTier, Severity};
|
||||
|
||||
/// Static catalog entry for a single diagnostic code.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct DiagnosticCode {
|
||||
/// The stable code string, e.g. `"OG-DS-104"`.
|
||||
pub code: &'static str,
|
||||
pub family: Family,
|
||||
pub tier: SafetyTier,
|
||||
pub default_severity: Severity,
|
||||
pub short: &'static str,
|
||||
}
|
||||
|
||||
// ─── Destructive (DS) — data-loss; always requires explicit opt-in ──────────
|
||||
|
||||
/// Reserved: dropping an entire graph (schema-level). Not yet emitted.
|
||||
pub const OG_DS_101: DiagnosticCode = DiagnosticCode {
|
||||
code: "OG-DS-101",
|
||||
family: Family::DS,
|
||||
tier: SafetyTier::Destructive,
|
||||
default_severity: Severity::Error,
|
||||
short: "drop graph type with rows",
|
||||
};
|
||||
|
||||
/// Drop a node type that has rows.
|
||||
pub const OG_DS_102: DiagnosticCode = DiagnosticCode {
|
||||
code: "OG-DS-102",
|
||||
family: Family::DS,
|
||||
tier: SafetyTier::Destructive,
|
||||
default_severity: Severity::Error,
|
||||
short: "drop node type with rows",
|
||||
};
|
||||
|
||||
/// Drop an edge type that has rows.
|
||||
pub const OG_DS_103: DiagnosticCode = DiagnosticCode {
|
||||
code: "OG-DS-103",
|
||||
family: Family::DS,
|
||||
tier: SafetyTier::Destructive,
|
||||
default_severity: Severity::Error,
|
||||
short: "drop edge type with rows",
|
||||
};
|
||||
|
||||
/// Drop a property (column) that has data.
|
||||
pub const OG_DS_104: DiagnosticCode = DiagnosticCode {
|
||||
code: "OG-DS-104",
|
||||
family: Family::DS,
|
||||
tier: SafetyTier::Destructive,
|
||||
default_severity: Severity::Error,
|
||||
short: "drop property with rows",
|
||||
};
|
||||
|
||||
/// Reserved: dropping a populated vector / embedding column. Distinct
|
||||
/// from a normal property drop because it invalidates downstream
|
||||
/// `nearest()` / `@embed` references.
|
||||
pub const OG_DS_105: DiagnosticCode = DiagnosticCode {
|
||||
code: "OG-DS-105",
|
||||
family: Family::DS,
|
||||
tier: SafetyTier::Destructive,
|
||||
default_severity: Severity::Error,
|
||||
short: "drop populated vector column",
|
||||
};
|
||||
|
||||
// ─── Maybe-fail (MF) — data-dependent; may fail on existing rows ────────────
|
||||
|
||||
/// Add a required (non-nullable) property to a populated type without
|
||||
/// `@default`. Existing rows have no value to fill in.
|
||||
pub const OG_MF_103: DiagnosticCode = DiagnosticCode {
|
||||
code: "OG-MF-103",
|
||||
family: Family::MF,
|
||||
tier: SafetyTier::Validated,
|
||||
default_severity: Severity::Error,
|
||||
short: "add required property without @default to populated type",
|
||||
};
|
||||
|
||||
/// Tighten nullable to non-nullable. May fail on existing null rows.
|
||||
/// Reserved for a follow-up that wires the validated-tier scan; not
|
||||
/// emitted in v0.
|
||||
pub const OG_MF_104: DiagnosticCode = DiagnosticCode {
|
||||
code: "OG-MF-104",
|
||||
family: Family::MF,
|
||||
tier: SafetyTier::Validated,
|
||||
default_severity: Severity::Error,
|
||||
short: "tighten nullable to non-nullable",
|
||||
};
|
||||
|
||||
/// Narrow a scalar (e.g. I64 → I32, F64 → F32). Lossy cast that may
|
||||
/// truncate or overflow. Today emitted on any `prop_type` change since
|
||||
/// the v1 planner doesn't yet distinguish widening from narrowing.
|
||||
pub const OG_MF_106: DiagnosticCode = DiagnosticCode {
|
||||
code: "OG-MF-106",
|
||||
family: Family::MF,
|
||||
tier: SafetyTier::Destructive,
|
||||
default_severity: Severity::Error,
|
||||
short: "narrowing scalar type",
|
||||
};
|
||||
|
||||
/// All v0 catalog entries. Used for chassis-level invariants
|
||||
/// (uniqueness, family coverage).
|
||||
pub const ALL_CODES: &[DiagnosticCode] = &[
|
||||
OG_DS_101, OG_DS_102, OG_DS_103, OG_DS_104, OG_DS_105, OG_MF_103, OG_MF_104, OG_MF_106,
|
||||
];
|
||||
|
||||
/// Codes actually emitted by the planner in v0 (i.e. not reserved).
|
||||
pub const EMITTED_IN_V0: &[&str] = &["OG-DS-102", "OG-DS-103", "OG-DS-104", "OG-MF-103", "OG-MF-106"];
|
||||
|
||||
/// Look up a code by its string identifier.
|
||||
pub fn lookup(code: &str) -> Option<&'static DiagnosticCode> {
|
||||
ALL_CODES.iter().find(|c| c.code == code)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn codes_are_unique() {
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
for c in ALL_CODES {
|
||||
assert!(seen.insert(c.code), "duplicate code: {}", c.code);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn code_strings_match_family_prefix() {
|
||||
for c in ALL_CODES {
|
||||
let expected_prefix = format!("OG-{}-", c.family.prefix());
|
||||
assert!(
|
||||
c.code.starts_with(&expected_prefix),
|
||||
"{} doesn't start with {}",
|
||||
c.code,
|
||||
expected_prefix
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn code_strings_have_three_digit_suffix() {
|
||||
for c in ALL_CODES {
|
||||
let suffix = c.code.rsplit('-').next().unwrap();
|
||||
assert_eq!(suffix.len(), 3, "{}: suffix not 3 chars", c.code);
|
||||
assert!(
|
||||
suffix.chars().all(|ch| ch.is_ascii_digit()),
|
||||
"{}: suffix not all digits",
|
||||
c.code
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn destructive_tier_defaults_to_error_severity() {
|
||||
for c in ALL_CODES {
|
||||
if c.tier == SafetyTier::Destructive {
|
||||
assert_eq!(
|
||||
c.default_severity,
|
||||
Severity::Error,
|
||||
"{}: destructive must default to Error",
|
||||
c.code
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lookup_finds_known_codes() {
|
||||
assert_eq!(lookup("OG-DS-104"), Some(&OG_DS_104));
|
||||
assert_eq!(lookup("OG-MF-103"), Some(&OG_MF_103));
|
||||
assert!(lookup("OG-XX-999").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn emitted_codes_exist_in_catalog() {
|
||||
for code in EMITTED_IN_V0 {
|
||||
assert!(
|
||||
lookup(code).is_some(),
|
||||
"EMITTED_IN_V0 contains {} but it isn't in ALL_CODES",
|
||||
code
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
83
crates/omnigraph-compiler/src/lint/diagnostic.rs
Normal file
83
crates/omnigraph-compiler/src/lint/diagnostic.rs
Normal file
|
|
@ -0,0 +1,83 @@
|
|||
//! Diagnostic types for the schema-lint chassis (MR-694).
|
||||
//!
|
||||
//! Every schema-migration diagnostic carries a stable code (`OG-XXX-NNN`),
|
||||
//! a family grouping (DS / MF / CD / …), and a safety tier
|
||||
//! (safe / validated / destructive). The code is the public identity;
|
||||
//! external tooling and operators reference rules by code, not by message.
|
||||
//!
|
||||
//! This module is the chassis-level vocabulary. The concrete code catalog
|
||||
//! lives in [`super::codes`]; emission sites are in
|
||||
//! `catalog::schema_plan` and (future) other lint passes.
|
||||
|
||||
/// Family groupings for schema-lint rules. Mirrors the Atlas analyzer
|
||||
/// families (DS / MF / CD / BC / NM) plus four omnigraph-native families
|
||||
/// for vector/embedding (VE), edge topology (ED), lock/cost (LK), and
|
||||
/// non-linear branch divergence (NL). Ownership (OW) is reserved for
|
||||
/// per-resource Cedar policy integration (MR-722).
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
|
||||
pub enum Family {
|
||||
/// Destructive — data-loss class. Always requires explicit opt-in.
|
||||
DS,
|
||||
/// Maybe-fail — data-dependent, may fail on existing rows.
|
||||
MF,
|
||||
/// Constraint deletion — invariant relaxation; consumer-warning.
|
||||
CD,
|
||||
/// Backward incompatible — rename or shape change that breaks clients.
|
||||
BC,
|
||||
/// Naming conventions.
|
||||
NM,
|
||||
/// Ownership — per-resource access control.
|
||||
OW,
|
||||
/// Non-linear — branch-merge schema-state divergence.
|
||||
NL,
|
||||
/// Vector / embedding — omnigraph-native.
|
||||
VE,
|
||||
/// Edge / graph topology — omnigraph-native.
|
||||
ED,
|
||||
/// Lock duration / cost — omnigraph-native.
|
||||
LK,
|
||||
}
|
||||
|
||||
impl Family {
|
||||
pub fn prefix(self) -> &'static str {
|
||||
match self {
|
||||
Self::DS => "DS",
|
||||
Self::MF => "MF",
|
||||
Self::CD => "CD",
|
||||
Self::BC => "BC",
|
||||
Self::NM => "NM",
|
||||
Self::OW => "OW",
|
||||
Self::NL => "NL",
|
||||
Self::VE => "VE",
|
||||
Self::ED => "ED",
|
||||
Self::LK => "LK",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Tier classification for a migration step. Determines apply-path
|
||||
/// behavior:
|
||||
/// - `Safe`: applies without scan or flag.
|
||||
/// - `Validated`: requires a single-pass scan of existing rows; fails on
|
||||
/// the first violation.
|
||||
/// - `Destructive`: requires explicit `--allow-data-loss` (or equivalent
|
||||
/// opt-in) at apply time.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
|
||||
pub enum SafetyTier {
|
||||
Safe,
|
||||
Validated,
|
||||
Destructive,
|
||||
}
|
||||
|
||||
/// Severity for a diagnostic at the user-facing surface. Defaults are set
|
||||
/// per code in [`super::codes`]; operators override via `omnigraph.yaml`
|
||||
/// (planned for a follow-up PR).
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
|
||||
pub enum Severity {
|
||||
/// Blocks apply.
|
||||
Error,
|
||||
/// Reported but doesn't block.
|
||||
Warn,
|
||||
/// Informational; doesn't block.
|
||||
Info,
|
||||
}
|
||||
28
crates/omnigraph-compiler/src/lint/mod.rs
Normal file
28
crates/omnigraph-compiler/src/lint/mod.rs
Normal file
|
|
@ -0,0 +1,28 @@
|
|||
//! Schema-lint chassis (MR-694).
|
||||
//!
|
||||
//! Stable diagnostic codes (`OG-XXX-NNN`) for schema-migration plans,
|
||||
//! the foundation for per-rule severity config, suppression directives,
|
||||
//! and pre-migration checks that subsequent PRs layer on.
|
||||
//!
|
||||
//! ## v0 surface
|
||||
//!
|
||||
//! - [`diagnostic`] defines [`Family`](diagnostic::Family),
|
||||
//! [`SafetyTier`](diagnostic::SafetyTier), and
|
||||
//! [`Severity`](diagnostic::Severity).
|
||||
//! - [`codes`] holds the catalog of [`DiagnosticCode`](codes::DiagnosticCode)
|
||||
//! entries; the planner attaches `code: Option<&'static str>` to each
|
||||
//! `UnsupportedChange` emission.
|
||||
//! - The CLI renders the code in `omnigraph schema plan` output; the
|
||||
//! apply path includes it in the user-visible error message.
|
||||
//!
|
||||
//! Future PRs add: severity config from `omnigraph.yaml`, `@allow(...)`
|
||||
//! suppression annotations, pre-migration checks (MR-941), the CD / VE /
|
||||
//! LK / NM families (MR-942..945), and CI integration (MR-946).
|
||||
//!
|
||||
//! See: docs/schema-lint.md, https://atlasgo.io/lint/analyzers
|
||||
|
||||
pub mod codes;
|
||||
pub mod diagnostic;
|
||||
|
||||
pub use codes::{lookup, DiagnosticCode, ALL_CODES};
|
||||
pub use diagnostic::{Family, SafetyTier, Severity};
|
||||
|
|
@ -53,15 +53,12 @@ pub(super) async fn apply_schema_with_lock(
|
|||
let plan = plan_schema_migration(&accepted_ir, &desired_ir)
|
||||
.map_err(|err| OmniError::manifest(err.to_string()))?;
|
||||
if !plan.supported {
|
||||
let reason = plan
|
||||
let message = plan
|
||||
.steps
|
||||
.iter()
|
||||
.find_map(|step| match step {
|
||||
SchemaMigrationStep::UnsupportedChange { reason, .. } => Some(reason.as_str()),
|
||||
_ => None,
|
||||
})
|
||||
.unwrap_or("unsupported schema migration plan");
|
||||
return Err(OmniError::manifest(reason.to_string()));
|
||||
.find_map(|step| step.unsupported_error_message())
|
||||
.unwrap_or_else(|| "unsupported schema migration plan".to_string());
|
||||
return Err(OmniError::manifest(message));
|
||||
}
|
||||
if plan.steps.is_empty() {
|
||||
return Ok(SchemaApplyResult {
|
||||
|
|
@ -141,8 +138,11 @@ pub(super) async fn apply_schema_with_lock(
|
|||
}
|
||||
SchemaMigrationStep::UpdateTypeMetadata { .. }
|
||||
| SchemaMigrationStep::UpdatePropertyMetadata { .. } => {}
|
||||
SchemaMigrationStep::UnsupportedChange { reason, .. } => {
|
||||
return Err(OmniError::manifest(reason.clone()));
|
||||
step @ SchemaMigrationStep::UnsupportedChange { .. } => {
|
||||
return Err(OmniError::manifest(
|
||||
step.unsupported_error_message()
|
||||
.unwrap_or_else(|| "unsupported schema migration step".to_string()),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -126,9 +126,10 @@ async fn apply_schema_rejects_dropping_a_property_with_data() {
|
|||
// the column is nullable — it would silently destroy data.
|
||||
let desired = TEST_SCHEMA.replace(" age: I32?\n", "");
|
||||
let err = db.apply_schema(&desired).await.unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
err.to_string().contains("removing property"),
|
||||
"expected 'removing property' in error, got: {err}"
|
||||
msg.contains("OG-DS-104"),
|
||||
"expected schema-lint code OG-DS-104 in error, got: {msg}"
|
||||
);
|
||||
|
||||
// Manifest didn't advance and existing rows are untouched.
|
||||
|
|
@ -166,8 +167,8 @@ edge Knows: Person -> Person {
|
|||
let err = db.apply_schema(desired).await.unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
msg.contains("removing node type") || msg.contains("removing edge type"),
|
||||
"expected drop-type error, got: {msg}"
|
||||
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}"
|
||||
);
|
||||
assert_eq!(
|
||||
db.snapshot_of(ReadTarget::branch("main"))
|
||||
|
|
@ -191,9 +192,10 @@ async fn apply_schema_rejects_dropping_an_edge_type() {
|
|||
// Drop only the `WorksAt` edge.
|
||||
let desired = TEST_SCHEMA.replace("\nedge WorksAt: Person -> Company", "");
|
||||
let err = db.apply_schema(&desired).await.unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
err.to_string().contains("removing edge type"),
|
||||
"expected 'removing edge type' error, got: {err}"
|
||||
msg.contains("OG-DS-103"),
|
||||
"expected schema-lint code OG-DS-103 in error, got: {msg}"
|
||||
);
|
||||
assert_eq!(
|
||||
db.snapshot_of(ReadTarget::branch("main"))
|
||||
|
|
@ -221,9 +223,10 @@ async fn apply_schema_rejects_adding_a_required_property_without_backfill() {
|
|||
" age: I32?\n email: String\n}",
|
||||
);
|
||||
let err = db.apply_schema(&desired).await.unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
err.to_string().contains("adding required property"),
|
||||
"expected 'adding required property' error, got: {err}"
|
||||
msg.contains("OG-MF-103"),
|
||||
"expected schema-lint code OG-MF-103 in error, got: {msg}"
|
||||
);
|
||||
assert_eq!(
|
||||
db.snapshot_of(ReadTarget::branch("main"))
|
||||
|
|
@ -253,8 +256,8 @@ async fn plan_schema_for_property_type_narrowing_is_not_supported() {
|
|||
assert!(!plan.supported, "narrowing I64 -> I32 must not be supported");
|
||||
assert!(plan.steps.iter().any(|step| matches!(
|
||||
step,
|
||||
SchemaMigrationStep::UnsupportedChange { reason, .. }
|
||||
if reason.contains("changing property type")
|
||||
SchemaMigrationStep::UnsupportedChange { code, .. }
|
||||
if code.as_deref() == Some("OG-MF-106")
|
||||
)));
|
||||
}
|
||||
|
||||
|
|
|
|||
61
docs/schema-lint.md
Normal file
61
docs/schema-lint.md
Normal file
|
|
@ -0,0 +1,61 @@
|
|||
# Schema lint
|
||||
|
||||
The migration planner emits **code-tagged diagnostics** for every schema change it rejects. Codes have the form `OG-XXX-NNN` and identify the rule (not the message); operators reference them in suppression directives, severity overrides, and CI reports.
|
||||
|
||||
This page is the catalog of codes shipped today. The chassis behind it is tracked in [MR-694](https://linear.app/modernrelay/issue/MR-694).
|
||||
|
||||
## What's shipped in v0
|
||||
|
||||
- Stable code attached to every rejection the planner emits (today: 5 of 17 paths — the rest carry `code: None` and are tagged as future work).
|
||||
- Code appears in the user-visible error message: `[OG-DS-104] removing property 'Person.age' is not supported …`.
|
||||
- CLI `omnigraph schema plan` shows the code on `unsupported change …` lines.
|
||||
- Tests in `tests/schema_apply.rs` assert on codes, not on free-text prose.
|
||||
|
||||
## What's not shipped yet
|
||||
|
||||
- Severity configuration in `omnigraph.yaml` (planned: `lint: { OG-DS-103: error }`).
|
||||
- `@allow(OG-XXX-NNN, "rationale")` suppression directives.
|
||||
- Pre-migration checks (the `migration_check { … }` block — MR-941).
|
||||
- The CD / VE / LK / NM families (MR-942..945).
|
||||
- CI integration (MR-946).
|
||||
- Cost-class annotations (MR-944).
|
||||
|
||||
See the parent chassis issue (MR-694) for the design and the per-family sub-issues for what's planned.
|
||||
|
||||
## Code catalog (v0)
|
||||
|
||||
The chassis defines ten families. Today only DS and MF have emitted codes. The remaining families are reserved for future PRs.
|
||||
|
||||
| Code | Family | Tier | Default severity | Meaning |
|
||||
|---|---|---|---|---|
|
||||
| `OG-DS-101` | Destructive | destructive | error | drop graph type with rows (reserved; not yet emitted) |
|
||||
| `OG-DS-102` | Destructive | destructive | error | drop node type with rows |
|
||||
| `OG-DS-103` | Destructive | destructive | error | drop edge type with rows |
|
||||
| `OG-DS-104` | Destructive | destructive | error | drop property with rows |
|
||||
| `OG-DS-105` | Destructive | destructive | error | drop populated vector column (reserved) |
|
||||
| `OG-MF-103` | Maybe-fail | validated | error | add required property without `@default` to populated type |
|
||||
| `OG-MF-104` | Maybe-fail | validated | error | tighten nullable to non-nullable (reserved) |
|
||||
| `OG-MF-106` | Maybe-fail | destructive | error | narrowing scalar type |
|
||||
|
||||
The full code catalog source of truth lives in `crates/omnigraph-compiler/src/lint/codes.rs`. CI-level invariants (uniqueness, format, family coverage) are unit-tested in the same module.
|
||||
|
||||
## Families
|
||||
|
||||
The ten chassis families:
|
||||
|
||||
| Prefix | Family | Status |
|
||||
|---|---|---|
|
||||
| **DS** | Destructive (data-loss) | shipped, v0 |
|
||||
| **MF** | Maybe-fail / data-dependent | shipped, v0 |
|
||||
| **CD** | Constraint deletion (relaxation warning) | tracked in MR-942 |
|
||||
| **BC** | Backward-incompatible (rename) | implicit in `@rename_from`; codify later |
|
||||
| **NM** | Naming conventions | tracked in MR-945 |
|
||||
| **OW** | Ownership (per-resource Cedar) | tracked in MR-722 |
|
||||
| **NL** | Non-linear (branch-merge divergence) | stubbed in MR-947 |
|
||||
| **VE** | Vector / embedding | tracked in MR-943 |
|
||||
| **ED** | Edge / graph topology | tracked in MR-701, MR-943 |
|
||||
| **LK** | Lock duration / cost | tracked in MR-944 |
|
||||
|
||||
## Prior art
|
||||
|
||||
The chassis is modeled on [Atlas's `sqlcheck` analyzers](https://atlasgo.io/lint/analyzers) (DS / MF / CD / BC / NM families). Atlas was the direct inspiration for stable codes, per-rule severity, suppression directives with rationale, and pre-migration checks. omnigraph adapts the chassis to a typed-IR substrate (no SQL injection vector, no per-engine locking, native vector / edge / embedding types Atlas doesn't have).
|
||||
Loading…
Add table
Add a link
Reference in a new issue