feat(wren-mdl): loosen manifest schema and bump layout version to 4 - #2501
Conversation
Widen column.properties value types to match model/relationship/view, and add optional annotation fields: manifest-level description + properties, model.uniqueKeys, and description + properties on cube measure/cubeDimension/timeDimension. All additive and backward-compatible. These give natively-typed homes for descriptions, business context, and uniqueness metadata (e.g. semantic-layer interop and knowledge annotations) without touching the Rust engine: the manifest structs don't set deny_unknown_fields, so unknown fields are ignored on deserialization. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughThe MDL schema now supports richer custom-property values, adds descriptions and root metadata, defines single-column or composite model unique keys, and introduces layout version 4 validation with a no-op migration from version 3. ChangesMDL schema and versioning
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The schema loosening in the previous commit is a new MDL layout revision, matching the precedent of v2 (optional dialect) and v3 (composite primaryKey), both additive no-op migrations. Bump MAX_SUPPORTED_LAYOUT_VERSION to 4 and add the no-op v3->v4 migration step so migrate_manifest stamps it correctly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/wren-core-base/src/mdl/migration.rs (1)
145-151: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winExercise representative v4 fields in the migration test.
The test currently verifies only
layoutVersion; include descriptions, nested properties, and compositeuniqueKeysto ensure the no-op migration preserves the new metadata and richer JSON values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/wren-core-base/src/mdl/migration.rs` around lines 145 - 151, Expand test_migrate_v3_to_v4 with representative v4 input fields, including a description, nested properties, and composite uniqueKeys. After migrate_manifest, assert these metadata and nested JSON values remain unchanged alongside the existing layoutVersion assertion, verifying the no-op migration preserves richer v4 content.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@core/wren-core-base/src/mdl/migration.rs`:
- Around line 145-151: Expand test_migrate_v3_to_v4 with representative v4 input
fields, including a description, nested properties, and composite uniqueKeys.
After migrate_manifest, assert these metadata and nested JSON values remain
unchanged alongside the existing layoutVersion assertion, verifying the no-op
migration preserves richer v4 content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d9e5685d-017f-4666-bd3d-356720127414
📒 Files selected for processing (2)
core/wren-core-base/src/mdl/manifest.rscore/wren-core-base/src/mdl/migration.rs
What
A new MDL layout revision (v4): additive, backward-compatible annotation fields on the manifest schema, plus the matching supported-layout-version bump.
Schema (
core/wren-mdl/mdl.schema.json)column.propertiesvalue types tostring | number | boolean | object | array | null, matchingmodel/relationship/view(column was string-only — an inconsistency).description+properties.model.uniqueKeys(a column or list of columns, likeprimaryKey).description+propertiesto cubemeasure/cubeDimension/timeDimension.Layout version (
core/wren-core-base)MAX_SUPPORTED_LAYOUT_VERSION3 → 4 and add the no-opmigrate_v3_to_v4step — matching the precedent of v2 (optionaldialect) and v3 (compositeprimaryKey), both additive no-op migrations.Why
Give natively-typed homes for human/agent descriptions, business context, and uniqueness metadata — useful for knowledge annotations and semantic-layer interop — instead of forcing everything through opaque string bags.
Compatibility
All changes are additive or validation-widenings; existing manifests stay valid and
v3→v4is a no-op. No query-engine / semantic-logic changes: the manifest structs don't set#[serde(deny_unknown_fields)], so unknown fields were already ignored — the only Rust delta is the version const + the no-op migration + tests.wren-core-base: 46 tests + clippy + fmt clean locally.🤖 Generated with Claude Code
Summary by CodeRabbit
descriptionplus flexible model properties and expanded custom property value types.uniqueKeyssupport for both single-column and composite uniqueness definitions.