Swiftdata Code Review

SwiftData Code Review

Quick Reference

Hard gates (before reporting findings)

Run in order; do not assert an issue until the gate for that issue passes.

1. Scope — pass when: You have the target .swift path(s) and confirmed SwiftData surface in scope (e.g. import SwiftData, @Model, @Query, @ModelActor, VersionedSchema, or migration types). If none apply, stop or narrow scope with one sentence. 2. Reference — pass when: For each checklist area you evaluate (models, queries, concurrency, migrations), you opened the matching references/*.md from the Quick Reference table or wrote N/A: no <area> in this review with a one-line reason. 3. Evidence — pass when: Every finding uses the [FILE:LINE] ISSUE_TITLE header (line range allowed) from the file you read; no finding without a cite. 4. Report — pass when: Findings list cites first (or inline) using [FILE:LINE] ISSUE_TITLE, then severity or checklist grouping—no uncited assertions.

Review Checklist

• [ ] Models marked final (subclassing crashes)
• [ ] @Relationship decorator on ONE side only (not both)
• [ ] Delete rules explicitly set (not relying on default .nullify)
• [ ] Relationships initialized to empty arrays, not default objects
• [ ] Batch operations used for bulk inserts (append(contentsOf:))
• [ ] @Query not loading thousands of items on main thread
• [ ] External values in predicates captured in local variables
• [ ] Scalar comparisons in predicates (not object references)
• [ ] @ModelActor used for background operations
• [ ] PersistentIdentifier/DTOs used to pass data between actors
• [ ] VersionedSchema defined for each shipped version
• [ ] MigrationPlan passed to ModelContainer

When to Load References

• Reviewing @Model or relationships -> model-design.md
• Reviewing @Query or #Predicate -> queries.md
• Reviewing @ModelActor or background work -> concurrency.md
• Reviewing schema changes or migrations -> migrations.md

Review Questions

1. Could this relationship assignment cause NULL foreign keys? 2. Is @Relationship on both sides creating circular references? 3. Could this @Query block the main thread with large datasets? 4. Are model objects being passed between actors unsafely? 5. Would schema changes require a migration plan?

SwiftData Concurrency

Best Practices

@ModelActor Pattern

@ModelActor
actor DataHandler {
    func importItems(_ data: [ImportData]) throws {
        for item in data {
            modelContext.insert(Item(name: item.name))
        }
        try modelContext.save()
    }

    func updateItem(id: PersistentIdentifier, name: String) throws {
        guard let item = self[id, as: Item.self] else { return }
        item.name = name
        try modelContext.save()
    }
}

Sendable DTO Pattern

struct ItemDTO: Sendable, Identifiable {
    let id: PersistentIdentifier
    let name: String
    let timestamp: Date
}

@ModelActor
actor DataService {
    func fetchItems() throws -> [ItemDTO] {
        try modelContext.fetch(FetchDescriptor<Item>())
            .map { ItemDTO(id: $0.persistentModelID, name: $0.name, timestamp: $0.timestamp) }
    }
}

Background Actor Creation

// GOOD: Ensures background execution
Task.detached {
    let handler = DataHandler(modelContainer: container)
    try await handler.importLargeDataset(data)
}

// Factory method alternative
@ModelActor
actor DataService {
    static nonisolated func createBackground(container: ModelContainer) async -> DataService {
        await Task.detached { DataService(modelContainer: container) }.value
    }
}

Critical Anti-Patterns

Passing Model Objects Between Actors

// BAD: Model objects are not Sendable
func getItem(id: UUID) throws -> Item {  // Crash or undefined behavior
    try modelContext.fetch(...).first!
}
let item = try await dataHandler.getItem(id: someId)
item.name = "New"  // Wrong thread!

// GOOD: Return DTO or use identifier
func getItemDTO(id: UUID) throws -> ItemDTO { ... }
func updateItem(id: PersistentIdentifier, name: String) throws { ... }

Creating Actor on Main Thread

// BAD: Actor runs on main thread
@MainActor
func setup() {
    let handler = DataHandler(modelContainer: container)
}

// GOOD: Create off main actor
func setup() async {
    await Task.detached {
        let handler = DataHandler(modelContainer: container)
    }.value
}

Single Actor Bottleneck

// BAD: All operations serialize
Task { try await sharedHandler.heavyImport1() }
Task { try await sharedHandler.heavyImport2() }  // Waits for import1!

// GOOD: Separate actors for independent work
Task.detached {
    let handler1 = DataHandler(modelContainer: container)
    try await handler1.heavyImport1()
}
Task.detached {
    let handler2 = DataHandler(modelContainer: container)
    try await handler2.heavyImport2()
}

Modifying After Actor Boundary

// BAD: Retaining models from background
let items = try await backgroundActor.fetchAllItems()  // [Item]
items[0].name = "Changed"  // CRASH - wrong context!

// GOOD: Use identifiers to load locally
let identifier = try await backgroundActor.getItemIdentifier()
await MainActor.run {
    let item = mainContext.model(for: identifier) as? Item
    item?.name = "Changed"
}

Missing @MainActor on Observable

// BAD: UI updates may happen off main thread
@Observable
class ViewModel {
    var items: [Item] = []
    func load() async {
        items = await dataService.fetchItems()  // May update from background
    }
}

// GOOD: Explicit main actor isolation
@Observable @MainActor
class ViewModel {
    var items: [Item] = []
}

Review Questions

• [ ] Is @ModelActor used for heavy data operations?
• [ ] Are model objects passed between actors? (They shouldn't be)
• [ ] Is Task.detached used for background actor creation?
• [ ] Is explicit save() called in Task contexts?
• [ ] Are ViewModels marked @Observable @MainActor?
• [ ] Are Sendable DTOs used for cross-actor data?
• [ ] Could a single actor become a bottleneck?
• [ ] Are PersistentIdentifiers used for cross-context references?

SwiftData Migrations

Best Practices

VersionedSchema Setup

enum UsersSchemaV1: VersionedSchema {
    static var versionIdentifier = Schema.Version(1, 0, 0)

    static var models: [any PersistentModel.Type] { [User.self] }

    @Model
    class User {
        var name: String
        init(name: String) { self.name = name }
    }
}

enum UsersSchemaV2: VersionedSchema {
    static var versionIdentifier = Schema.Version(2, 0, 0)

    static var models: [any PersistentModel.Type] { [User.self] }

    @Model
    class User {
        @Attribute(originalName: "name") var fullName: String  // Renamed
        @Attribute(.unique) var email: String  // Added
        init(fullName: String, email: String) { ... }
    }
}

typealias User = UsersSchemaV2.User

Migration Plan

enum UsersMigrationPlan: SchemaMigrationPlan {
    static var schemas: [any VersionedSchema.Type] {
        [UsersSchemaV1.self, UsersSchemaV2.self]
    }

    static var stages: [MigrationStage] { [migrateV1toV2] }

    // Custom: clean duplicates before adding .unique
    static let migrateV1toV2 = MigrationStage.custom(
        fromVersion: UsersSchemaV1.self,
        toVersion: UsersSchemaV2.self,
        willMigrate: { context in
            let users = try context.fetch(FetchDescriptor<UsersSchemaV1.User>())
            var seen = Set<String>()
            for user in users where seen.contains(user.email) {
                context.delete(user)
            }
            try context.save()
        },
        didMigrate: nil
    )

    // Lightweight: simple changes
    static let migrateV2toV3 = MigrationStage.lightweight(
        fromVersion: UsersSchemaV2.self,
        toVersion: UsersSchemaV3.self
    )
}

ModelContainer Configuration

let container = try ModelContainer(
    for: User.self,
    migrationPlan: UsersMigrationPlan.self
)

Critical Anti-Patterns

Renaming Without originalName

// BAD: Data LOST
var fullName: String  // Was "name", no mapping

// GOOD: Data preserved
@Attribute(originalName: "name") var fullName: String

Adding .unique to Duplicated Data

// BAD: Crash if duplicates exist
@Attribute(.unique) var email: String

// GOOD: Clean duplicates in willMigrate first
willMigrate: { context in
    // Remove duplicates before schema applies .unique
}

Custom Migrations with CloudKit

// BAD: Crashes with CloudKit
MigrationStage.custom(willMigrate: { ... }, didMigrate: { ... })

// GOOD: Lightweight only for CloudKit apps
MigrationStage.lightweight(fromVersion:, toVersion:)
// Handle complex logic in app initialization

Wrong Schema in Migration Closure

// BAD: V2 not available in willMigrate
willMigrate: { context in
    try context.fetch(FetchDescriptor<SchemaV2.User>())  // WRONG
}

// GOOD: V1 in willMigrate, V2 in didMigrate
willMigrate: { context in
    try context.fetch(FetchDescriptor<SchemaV1.User>())  // Correct
}
didMigrate: { context in
    try context.fetch(FetchDescriptor<SchemaV2.User>())  // Correct
}

Missing MigrationPlan

// BAD: Migration not applied
let container = try ModelContainer(for: User.self)

// GOOD: Migration plan specified
let container = try ModelContainer(
    for: User.self,
    migrationPlan: UsersMigrationPlan.self
)

Lightweight vs Custom

Lightweight migrations support:

• Adding properties with default values
• Renaming with @Attribute(originalName:)
• Deleting properties
• Adjusting delete rules

Custom migrations needed for:

• Data transformation
• Deduplication before .unique
• Complex relationship changes
• Default value calculation from existing data

Review Questions

• [ ] Is every shipped schema wrapped in VersionedSchema?
• [ ] Does each schema have a unique versionIdentifier?
• [ ] Are schemas listed in chronological order?
• [ ] Is there a MigrationStage for each version transition?
• [ ] Is MigrationPlan passed to ModelContainer?
• [ ] Do renamed properties use @Attribute(originalName:)?
• [ ] If adding .unique, are duplicates handled in willMigrate?
• [ ] Does willMigrate only access old schema models?
• [ ] Is CloudKit enabled? (Custom migrations will crash)
• [ ] Is context.save() called in migration closures?

SwiftData Model Design

Best Practices

@Attribute Options

Delete Rules

@Relationship(deleteRule: .cascade)    // Deletes children (owned relationships)
@Relationship(deleteRule: .nullify)    // Sets to nil (default, fragile)
@Relationship(deleteRule: .deny)       // Prevents deletion if children exist
@Relationship(deleteRule: .noAction)   // Does nothing (dangerous!)

Critical Anti-Patterns

Decorating Both Sides of Relationship

// BAD: Circular reference error
@Model class Student {
    @Relationship(inverse: \TestResult.student) var testResults: [TestResult]
}
@Model class TestResult {
    @Relationship(inverse: \Student.testResults) var student: Student?
}

// GOOD: @Relationship on one side only
@Model class Student {
    @Relationship(deleteRule: .cascade, inverse: \TestResult.student)
    var testResults: [TestResult] = []
}
@Model class TestResult {
    var student: Student?  // No decorator
}

Assignment in Initializer

// BAD: Foreign keys become NULL
init(floors: [Floor]) {
    self.floors = floors  // Bypasses tracking!
}

// GOOD: Use append
init(floors: [Floor]) {
    self.floors.append(contentsOf: floors)
}

Default Values for Relationships

// BAD: Runtime crash
var tag: Tag = Tag(name: "default")

// GOOD: Optional or set after insertion
var tag: Tag?

Individual Appends in Loop

// BAD: 700x slower than Core Data
for i in 0..<1000 {
    item.tags.append(Tag(name: "\(i)"))
}

// GOOD: Batch operation
var tags = (0..<1000).map { Tag(name: "\($0)") }
item.tags.append(contentsOf: tags)

Arrays for Searchable Data

// BAD: Stored as blob, not searchable
var tags: [String]

// GOOD: Relationship model
@Relationship(deleteRule: .cascade) var tags: [Tag] = []

Unique with CloudKit

// BAD: Breaks CloudKit sync
@Attribute(.unique) var email: String

// GOOD: Handle uniqueness programmatically
var email: String

Review Questions

• [ ] Is the model marked final?
• [ ] Does it have an explicit initializer?
• [ ] Are relationships var, not let?
• [ ] Is @Relationship on only ONE side of bidirectional relationships?
• [ ] Does delete rule match optionality?
• [ ] Are relationships initialized to = []?
• [ ] Are batch operations used for bulk additions?
• [ ] If using CloudKit, are there any .unique attributes?

SwiftData Queries

Best Practices

@Query Patterns

// Static query
@Query var items: [Item]

// Sorted query
@Query(sort: \Item.timestamp, order: .reverse) var items: [Item]

// Multi-field sort
@Query(sort: [SortDescriptor(\Item.priority, order: .reverse),
              SortDescriptor(\Item.name)]) var items: [Item]

// Dynamic filtering in init
init(showCompleted: Bool) {
    let completed = showCompleted  // Capture external value
    _items = Query(filter: #Predicate<Item> { item in
        completed || !item.isCompleted
    })
}

FetchDescriptor Patterns

var descriptor = FetchDescriptor<Item>(
    predicate: #Predicate { $0.createdAt > cutoffDate },
    sortBy: [SortDescriptor(\.createdAt, order: .reverse)]
)
descriptor.fetchLimit = 50
descriptor.fetchOffset = page * 50
descriptor.propertiesToFetch = [\.id, \.title]

let count = try context.fetchCount(descriptor)  // Count without fetching
let items = try context.fetch(descriptor)

#Index (iOS 18+)

@Model final class Order {
    #Index<Order>(
        [\.status],              // Filter by status
        [\.createdAt],           // Sort by date
        [\.status, \.createdAt]  // Compound index
    )
    var status: String
    var createdAt: Date
}

Critical Anti-Patterns

External Values Directly in @Query

// BAD: Date.now evaluated at macro expansion
@Query(filter: #Predicate<Event> { $0.date > Date.now })
var events: [Event]

// GOOD: Capture in local variable
static var now: Date { Date.now }
@Query(filter: #Predicate<Event> { $0.date > now })
var events: [Event]

Object Comparison in Predicate

// BAD: Runtime crash
let predicate = #Predicate<Post> { $0.author == selectedUser }

// GOOD: Compare identifier
let userId = selectedUser.id
let predicate = #Predicate<Post> { $0.author.id == userId }

Unsupported String Methods

// BAD: These crash
item.name.hasPrefix("A")     // Use starts(with:)
item.name.hasSuffix("z")     // Not supported
item.name.uppercased()       // Not translatable

// GOOD: Supported methods
item.name.starts(with: "A")
item.name.localizedStandardContains("test")

Wrong Boolean Comparison

// BAD: Runtime crash
#Predicate<Movie> { $0.cast.isEmpty == false }

// GOOD: Negation operator
#Predicate<Movie> { !$0.cast.isEmpty }

Fetching Just to Count

// BAD: Loads all objects into memory
let count = try context.fetch(FetchDescriptor<Item>()).count

// GOOD: Dedicated count method
let count = try context.fetchCount(FetchDescriptor<Item>())

Heavy @Query on Main Thread

// BAD: Freezes UI
@Query var allPhotos: [Photo]  // 10,000+ items

// GOOD: Pagination in view model
var descriptor = FetchDescriptor<Photo>()
descriptor.fetchLimit = 50
descriptor.fetchOffset = offset

Review Questions

• [ ] Is @Query loading only what the view needs?
• [ ] Are external values captured in local variables?
• [ ] Are comparisons using scalars, not object references?
• [ ] Are string methods limited to supported ones?
• [ ] Is !isEmpty used instead of isEmpty == false?
• [ ] Is fetchCount() used instead of fetching for counts?
• [ ] Are fetchLimit/fetchOffset used for pagination?
• [ ] Are frequently filtered properties indexed (iOS 18+)?