Checks and fixes code quality issues in Swift: fallback values that hide failures, backwards-compatibility shims for unreleased code, raw String/[String:Any] types where typed models should exist, optional properties that should be non-optional, AI-changelog comments that describe what changed rather than why, duplicated logic across call sites, inline string formatting for derived identifiers (branch names, keys, paths), raw string literals used as shared identifiers, and force unwraps. Use when reviewing Swift code for quality, before committing, when asked to clean up or harden code, or when ai-dev-tools-enforce is running.
Checks and fixes code quality issues in Swift: fallback values that hide failures, backwards-compatibility shims for unreleased code, raw String/[String:Any] types where typed models should exist, optional properties that should be non-optional, AI-changelog comments that describe what changed rather than why, duplicated logic across call sites, inline string formatting for derived identifiers (branch names, keys, paths), raw string literals used as shared identifiers, and force unwraps. Use when reviewing Swift code for quality, before committing, when asked to clean up or harden code, or when ai-dev-tools-enforce is running.
user-invocable
true
Code Quality
Your job is to fix code quality issues, not write a review. When you find a violation, make the change.
Fallback Values That Hide Failures
Fallback values can mask bugs — the caller never learns that the operation failed.
Look for:
?? "", ?? [], ?? 0, ?? false on paths that should never be nil
guard ... else { return } silently exiting when failure should be reported
try? expr ?? defaultValue discarding the error silently
Fix: Replace fallbacks with throws, guard let ... else { throw ... }, or a precondition/fatalError with a clear message. Only keep a fallback if the default value is semantically correct (e.g., a preference with a genuine default).
Backwards-Compatibility Shims for Unreleased Code
There is no backwards-compatibility obligation for code that has not shipped.
Look for:
Deprecated typealias pointing to a new name
@available(*, deprecated, renamed: "...") on types or methods that haven't shipped
Old method signatures kept alongside new ones with a // TODO: remove comment
Adapter or wrapper types that exist solely to bridge an old interface to a new one within the same codebase
Fix: Delete the shim entirely. Update all call sites to use the new API directly. If the call sites are in the same PR, there is no reason to keep both.
Raw String, [String: Any], and Untyped Dictionaries in APIs
Look for:
Method signatures that accept or return String where a more specific type exists
[String: Any] or [String: String] dictionaries
Any parameters
JSON-decoded results stored as dictionaries rather than Decodable structs
Fix: Define a struct or enum with named, typed fields. For identifiers, consider struct FooID: RawRepresentable rather than a bare String to prevent mixing up different ID spaces. For JSON responses, define a Decodable model.
Optional Where Value Must Be Present
Optional should mean "this value genuinely may not exist." It should not mean "I wasn't sure how to get this at initialization time."
Look for:
var foo: Foo? on properties always set during initialization
Optional return types on methods that always return a value
init parameters that are optional but required for the type to function
Fix:
Value known at init time → non-optional stored property, require it in init
Value not available at init → failable init? or throws rather than storing optional and checking it everywhere
Value required for an operation → guard let at the call site and throw or return an error rather than silently doing nothing when nil
AI-Changelog-Style Comments
Comments that describe what changed rather than what the code does become noise immediately after the PR merges.
Look for:
// Changed X to Y for new behavior
// Added Z to support the new flow
// Previously this was a class, now it's a struct
// Updated to use the new API
// Removed old implementation
Fix: Delete the comment. If something genuinely needs explanation (a non-obvious algorithm, a workaround for a known bug, a performance trade-off), replace it with a comment that explains why the code does what it does, not what it used to do.
Hidden Side Effects in Property Observers
A didSet or willSet that performs I/O or calls a use case is a hidden side effect — callers setting the property don't expect work to happen.
Look for:didSet { useCase.save(...) }, didSet { fileManager.write(...) }, or any didSet block that does more than update derived local state.
Fix: Remove the side effect from didSet and expose an explicit method. Callers that need the side effect call the method; callers that just want to set the value aren't surprised:
Look for: A struct with a hand-written init that exactly matches what Swift synthesizes — same parameter names, same types, same assignments, no defaulted parameters, no extra logic.
Fix: Delete the explicit init. Swift's synthesized memberwise initializer is identical and is always in sync with property additions or removals.
Duplicated Logic
Look for:
The same condition or predicate written in multiple places
The same sequence of operations repeated across two use cases or services
Identical switch arms or if/else chains in multiple files
Fix: Extract into a single function, computed property, or type that all sites call. For structural duplication across use cases, consider whether shared logic belongs in a Service. Do not create an abstraction for only two instances if they are likely to diverge — use judgment.
Inline String Formatting for Derived Identifiers
String formatting logic for derived values — branch names, cache keys, file paths, URL paths — must live in one place. When callers construct these strings themselves, small differences in format accumulate silently and become hard to find.
Fix: Extract to a static func on the type that owns the concept. All callers use that method — no one builds the string themselves:
// BEFORE (inline in PlanModel, and potentially duplicated elsewhere)let identifier = hashString(stem)
let branchName ="plan-\(identifier)"// AFTER — single definition, all callers use it// In PlanService (Features layer):publicstaticfuncworktreeBranchName(forplanURL: URL) -> String {
let stem = planURL.deletingPathExtension().lastPathComponent
// ... hash ...return"plan-\(identifier)"
}
// In PlanModel:let branchName =PlanService.worktreeBranchName(for: plan.planURL)
Raw String Literals as Shared Identifiers
A string literal used in multiple places as an identifier (feature name, path component, dictionary key) is a coordination hazard. If one call site changes spelling, the others silently diverge.
Fix: Define a named constant — a static var on the owning type, a static let on the relevant service or model, or a dedicated enum of cases. All callers reference the constant, not the literal:
// BEFORE
dataPathsService.path(for: .worktrees(feature: "claude-chain"))
dataPathsService.path(for: .worktrees(feature: "plan"))
// AFTER — defined once on ServicePathpublicextensionServicePath {
staticvar claudeChainWorktrees: ServicePath { .worktrees(feature: "claude-chain") }
staticvar planWorktrees: ServicePath { .worktrees(feature: "plan") }
}
// Callers
dataPathsService.path(for: .claudeChainWorktrees)
dataPathsService.path(for: .planWorktrees)
Force Unwraps
A force unwrap is a bet that a condition is impossible. When that bet is wrong, it crashes in production.
Look for:foo!, try!, as! in non-test, non-IBOutlet production code.
Fix:
foo! → guard let foo else { throw FooError.missing } or guard let foo else { return } with an appropriate error log
try! → try with throws on the enclosing function, or do/catch that sets error state
as! → as? with explicit handling of the nil case, or reconsider the type hierarchy so the cast is unnecessary
