Skip to content

Refactor: Make Store Optional - Node trait accepts any store type#2

Merged
nassor merged 8 commits intomainfrom
refactor/optional-store
Jul 25, 2025
Merged

Refactor: Make Store Optional - Node trait accepts any store type#2
nassor merged 8 commits intomainfrom
refactor/optional-store

Conversation

@nassor
Copy link
Copy Markdown
Owner

@nassor nassor commented Jul 25, 2025

This PR fundamentally refactors Cano's Node trait to remove the Store trait requirement, making data stores optional and enabling any type to be used as a store. This change significantly improves flexibility, performance, and usability.

🎯 Key Changes

1. Node Trait Refactoring

  • Removed Store trait bound: Node<TState, TParams, TStore> now accepts any store type
  • Improved generic naming: TTState, ParamsTParams, STStore for clarity
  • Enhanced flexibility: Enables direct struct access, custom stores, and stack allocation

2. Store Trait Renamed to KeyValueStore

  • Renamed: Store trait → KeyValueStore trait for clarity
  • Repositioned: Now positioned as optional helper utilities, not core requirements
  • Documentation rewrite: Emphasizes convenience rather than necessity
  • Clean break: Removed all compatibility aliases for cleaner API

3. New Custom Store Example

  • Added: workflow_stack_store.rs (334 lines)
  • Demonstrates: Custom RequestCtx struct as store type
  • Shows: Direct field access, type safety, performance benefits
  • Architecture: MetricsNode → ResponseNode communication pipeline

4. API Improvements

  • Workflow/Scheduler: Updated to handle generic store types with proper bounds
  • Error handling: Enhanced error types for better debugging
  • Type safety: Improved compile-time guarantees throughout

🔧 Technical Details

Before (Store trait required)

#[async_trait]
impl Node<WorkflowState> for MyNode {
    async fn prep(&self, store: &impl Store) -> Result<String, CanoError> {
        let data: String = store.get("key")?;  // Hash map lookup
        Ok(data)
    }
}

After (Any store type)

// Option 1: Use MemoryStore (KeyValueStore helper)
#[async_trait]
impl Node<WorkflowState, (), MemoryStore> for MyNode {
    async fn prep(&self, store: &MemoryStore) -> Result<String, CanoError> {
        let data: String = store.get("key")?;  // Still works
        Ok(data)
    }
}

// Option 2: Use custom store (better performance)
#[async_trait]
impl Node<WorkflowState, (), RequestCtx> for MyNode {
    async fn prep(&self, store: &RequestCtx) -> Result<String, CanoError> {
        Ok(store.request_body.clone())  // Direct field access
    }
}

📊 Performance Benefits

  • Direct field access: No hash map overhead for custom stores
  • Stack allocation: Custom stores can avoid heap allocations
  • Compile-time safety: Type checking for store structure
  • Zero abstraction cost: When using custom store types

📚 Updated Documentation

  • README.md: Comprehensive Store section rewrite explaining both approaches
  • Examples: All 13 examples updated to demonstrate new patterns
  • API docs: Enhanced documentation for Node trait and KeyValueStore helpers

🧪 Testing & Compatibility

  • All tests pass: 114/114 tests successful
  • All examples compile: 13 examples updated and verified
  • Breaking change: Clean API break for better long-term design
  • No warnings: Clean compilation across all targets

📁 Files Changed (22 files, +853/-537 lines)

Core Architecture:

  • node.rs - Node trait refactoring
  • workflow.rs - Generic store support
  • scheduler.rs - Type parameter updates
  • mod.rs - Store → KeyValueStore + documentation rewrite

Examples & Documentation:

  • workflow_stack_store.rs - NEW custom store demonstration
  • README.md - Store section rewrite with custom store examples
  • All 12 existing examples updated for new API

Supporting Changes:

  • lib.rs - Export updates, compatibility removal
  • error.rs - Enhanced error handling
  • Benchmarks updated for new patterns

🎉 Migration Path

Existing MemoryStore usage continues to work:

// This still works exactly the same
let store = MemoryStore::new();
store.put("key", value)?;
let result: Type = store.get("key")?;

New custom store patterns available:

// New: Use any struct as store
#[derive(Default)]
struct MyStore {
    pub data: String,
    pub count: i32,
}
// Direct field access, better performance

🔗 Related

This change enables future enhancements like:

  • Database-backed stores
  • Remote store implementations
  • Specialized pipeline data structures
  • Zero-copy processing patterns

Closes: Enhanced flexibility and performance for Cano workflows
Type: Breaking change (major version bump required)
Testing: ✅ All tests pass, examples verified

Copilot AI review requested due to automatic review settings July 25, 2025 21:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors Cano's Node trait to remove the Store trait requirement, making data stores optional and enabling any type to be used as a store. This improves flexibility by allowing direct struct field access instead of requiring key-value store patterns.

  • Node trait now accepts any store type instead of requiring Store trait implementation
  • Store trait renamed to KeyValueStore and repositioned as optional helper utility
  • New custom store example demonstrates using RequestCtx struct for direct field access

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/workflow.rs Updates Workflow to support generic store types with new type parameter naming
src/store/mod.rs Renames Store trait to KeyValueStore and rewrites documentation
src/store/memory.rs Updates MemoryStore to implement KeyValueStore instead of Store
src/store/error.rs Updates error messages to use TState instead of T generic naming
src/scheduler.rs Updates Scheduler to handle generic store types with new bounds
src/node.rs Removes Store trait requirement from Node trait, allowing any store type
src/lib.rs Updates exports to use KeyValueStore instead of Store
src/error.rs Updates type alias naming for consistency
examples/workflow_stack_store.rs New example demonstrating custom store types with RequestCtx
examples/*.rs Updates all examples to use new API patterns
benches/*.rs Updates benchmarks for new KeyValueStore trait
README.md Comprehensive rewrite of Store section with custom store examples
Cargo.toml Adds new workflow_stack_store example
Comments suppressed due to low confidence (2)

src/workflow.rs:301

  • The parameter name _store indicates it's unused, but the next line accesses _store.get(). The underscore prefix should be removed since the parameter is actually used.
        async fn prep(&self, _store: &MemoryStore) -> Result<Self::PrepResult, CanoError> {

src/workflow.rs:303

  • Using _store when the variable is actually being used is misleading. Should be store instead of _store.
            let data = _store

Comment thread src/store/mod.rs Outdated
Comment thread examples/workflow_simple.rs Outdated
nassor and others added 2 commits July 25, 2025 17:48
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@nassor nassor self-assigned this Jul 25, 2025
@nassor nassor merged commit 638f15d into main Jul 25, 2025
4 checks passed
@nassor nassor deleted the refactor/optional-store branch July 26, 2025 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants