Review Changes

Review and Fix Local Changes

Review uncommitted changes against the coding standards and patterns defined in this repository, then fix any issues found.

Arguments

• --subagent: Run in a subagent (isolates context, runs in background)

Mode Selection

Check if --subagent is in the arguments:

• If --subagent is present: Use the Task tool with subagent_type: "general-purpose" to run this review in a subagent, passing all other arguments
• If --subagent is NOT present: Run directly in current context (default)

Context

Current git diff to review: !git diff --no-color

Changed files: !git diff --name-only

Review Checklist

Analyze the diff above and check for the following issues:

1. Import Patterns

• No inline imports: All imports must be at the top of the file, never inside functions
• No full paths inline: Never use storage::DatabaseClient::new() inline; import types first
• Import order: Standard library first, then external crates, then local crates, then pub use re-exports

2. Naming Conventions

• Files/modules: snake_case (e.g., asset_id.rs)
• Functions/variables: snake_case
• Structs/enums: PascalCase
• Constants: SCREAMING_SNAKE_CASE
• No generic names: Avoid util, utils, normalize, or similar vague names
• Concise helper names: Within a module, use scope-reliant names (prefer is_spot_swap over is_hypercore_spot_swap)
• No type suffixes: Avoid _str, _int, _vec suffixes; Rust's type system makes them redundant

3. Error Handling

• Prefer plain Error: Use plain Error types, not thiserror macros
• Implement From traits: For error conversion between types
• Propagate with ?: Prefer ? operator over manual map_err where possible
• Consistent Result<T, Error>: Use consistent return types
• Constructor methods on errors: Use ErrorType::constructor(msg) instead of verbose ErrorType::Variant("redundant context".into())

3b. JSON Parameter Extraction

• Use primitives::ValueAccess: For serde_json::Value access, use composable trait methods (get_value(key), at(index), string()) instead of manual .get().ok_or() chains. Chain for compound access: params.get_value("key")?.at(0)?.string()?
• Accessor methods on parent types: Add accessor methods (e.g., TransactionLoadInput::get_data_extra()) to avoid pattern-matching boilerplate at call sites

4. Code Style

• Line length: Maximum 180 characters
• Avoid matches!: Don't use matches! for pattern matching; it's easy to miss cases later
• No over-engineering: Only make changes directly requested or clearly necessary
• No docstrings/comments/annotations: Don't add docstrings, comments, or /// docs unless explicitly asked; remove any that were added (including in mod.rs files)
• No #[allow(dead_code)]: Remove dead code instead of suppressing warnings; if code is needed, use it
• No unused fields: Remove unused fields from structs/models; don't keep fields "for future use"
• Constants for magic numbers: Extract magic numbers into named constants with clear meaning
• Minimum interface: Don't expose unnecessary functions; if client only needs one function, don't add multiple variants
• Use uniffi::remote: For UniFFI wrapper types around external models, use #[uniffi::remote] instead of creating duplicate structs with From implementations:

  // Record example
  use primitives::AuthNonce;
  pub type GemAuthNonce = AuthNonce;
  #[uniffi::remote(Record)]
  pub struct GemAuthNonce { pub nonce: String, pub timestamp: u32 }
  
  // Enum example
  use primitives::SwapperMode;
  pub type GemSwapperMode = SwapperMode;
  #[uniffi::remote(Enum)]
  pub enum GemSwapperMode { ExactIn, ExactOut }

• Simple solutions: Three similar lines is better than a premature abstraction
• Avoid mut: Prefer immutable bindings; use mut only when truly necessary
• Prefer one-liners: Inline single-use variables; avoid creating variables used only once
• Avoid #[serde(default)]: Only use when the field is genuinely optional in the API response; if the field is always present, omit it
• Use accessor methods for enum variants: Instead of destructuring enum variants with match, use typed accessor methods (e.g., metadata.get_sequence()? instead of match &metadata { Cosmos { sequence, .. } => ... })

5. Code Organization

• Modular structure: Break down files into smaller, focused modules; separate models from clients/logic (e.g., models.rs + client.rs, not everything in one file)
• Folder modules for complexity: When a module has multiple concerns (models, client, mappers), use a folder with mod.rs instead of a single file
• Avoid duplication: Search for existing implementations before writing new code; reuse existing code or crates
• Shared crates: Reusable logic belongs in shared crates (e.g., gem_solana, gem_evm), not in utility binaries; move shared code to appropriate crates
• Bird's eye view: Step back and identify opportunities to simplify and consolidate

6. Async Patterns

• Tokio runtime: Use tokio for async operations
• Shared state: Use Arc<tokio::sync::Mutex<T>> for shared async state
• Async client structs: Should return Result<T, Error>