chore(aiagent): store memory in db

[centdix]

No description provided.

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	57fc166
Status:	✅  Deploy successful!
Preview URL:	https://4a890de5.windmill.pages.dev
Branch Preview URL:	https://fg-memory32.windmill.pages.dev

View logs

[centdix]

/updatesqlx

[windmill-internal-app]

Starting sqlx update...

[windmill-internal-app]

❌ SQLx update failed. Please check the workflow logs for details.

[centdix]

/updatesqlx

[windmill-internal-app]

Starting sqlx update...

[windmill-internal-app]

❌ SQLx update failed. Please check the workflow logs for details.

[centdix]

/updatesqlx

[windmill-internal-app]

Starting sqlx update...

[windmill-internal-app]

❌ SQLx update failed. Please check the workflow logs for details.

[claude]

Claude finished @centdix's task —— View job

---

PR Review: AI Agent Memory Storage ✅

Review Complete

---

Overview

This PR implements database storage for AI agent memory as a fallback when S3 is not configured. The implementation adds a new ai_agent_memory table and modifies the memory handling logic to support both S3 and database storage.

Database Migration Review

✅ Migration Structure (backend/migrations/20251104135208_ai_agent_memory.up.sql:4-12)

The table structure looks solid:

• Composite primary key (workspace_id, conversation_id, step_id) is appropriate
• JSONB type for messages provides flexibility
• Timestamps with timezone are correctly used

⚠️ Potential Issue: Index Missing

Concern: No index for common query patterns. The read_from_db function queries by (workspace_id, conversation_id, step_id), which matches the primary key. However, delete_conversation_from_db queries by (workspace_id, conversation_id) without step_id.

Recommendation: Consider adding an index for deletion queries:

CREATE INDEX idx_ai_agent_memory_conversation 
ON ai_agent_memory (workspace_id, conversation_id);

Backend Rust Code Review

✅ Well-Structured Memory Management (backend/windmill-worker/src/memory_common.rs)

Strengths:

1. Size limiting: The 100KB limit with truncation is a good safeguard (lines 5, 50-63)
2. Clear separation: OSS vs Enterprise handling is well separated
3. UPSERT pattern: Proper use of ON CONFLICT for idempotent writes (lines 66-78)

⚠️ Performance Consideration: Truncation Algorithm (backend/windmill-worker/src/memory_common.rs:103-123)

Issue: The truncation algorithm has O(n²) complexity - it serializes the entire message array on each iteration:

while !result.is_empty() {
    let test_json = serde_json::to_value(&result)?;  // Serializes entire array
    let test_size = test_json.to_string().len();     // Converts to string
    
    if test_size <= max_size_bytes {
        break;
    }
    result.remove(0);  // Also O(n) for Vec
}

Recommendation: For better performance with large message arrays:

// Binary search approach
let mut left = 0;
let mut right = messages.len();
while left < right {
    let mid = (left + right + 1) / 2;
    let test_messages = &messages[messages.len() - mid..];
    let test_json = serde_json::to_value(test_messages)?;
    if test_json.to_string().len() <= max_size_bytes {
        left = mid;
    } else {
        right = mid - 1;
    }
}
Ok(messages[messages.len() - left..].to_vec())

This reduces complexity from O(n²) to O(n log n).

⚠️ Error Handling in Async Context (backend/windmill-api/src/flow_conversations.rs:187-205)

Issue: Memory deletion errors are logged but not surfaced to the user:

tokio::spawn(async move {
    if let Err(e) = windmill_worker::memory_oss::delete_conversation_memory(...).await {
        tracing::error!("Failed to delete memory for conversation {}: {:?}", ...)
    }
});

Impact: Users won't know if memory cleanup failed, potentially leading to data leaks or storage issues.

Recommendation: Consider either:

1. Awaiting the deletion (if performance permits)
2. Returning a warning in the response
3. Adding a background job retry mechanism

✅ Good Warning Message (backend/windmill-worker/src/memory_common.rs:52-59)

The truncation warning message is helpful and guides users toward S3 storage.

Frontend Review

✅ Svelte 5 Best Practices (frontend/src/lib/components/InputTransformForm.svelte:162-172)

Good: The new 'ai' property type is well integrated:

if (isAgentTool && ((arg?.type === 'static' && arg?.value === undefined) || arg?.type === 'ai')) {
    if (arg?.type === 'static') {
        arg.type = 'ai'
    }
    return 'ai'
}

⚠️ Minor: UI Message Could Be More Informative (frontend/src/lib/components/InputTransformForm.svelte:721-728)

The message "This field will be filled by the AI agent dynamically" is clear, but could mention that this happens at runtime during tool execution.

✅ Schema Integration (frontend/src/lib/components/flows/flowInfers.ts:36-42)

The warning message for S3 storage is excellent and informative:

'x-no-s3-storage-workspace-warning':
    'When no S3 storage is configured in your workspace settings, memory will be stored in database, which implies a limit of 100KB per memory entry...'

Security Considerations

✅ Proper Permissions

• Database permissions are correctly granted to windmill_admin and windmill_user
• No SQL injection vulnerabilities (parameterized queries used throughout)

✅ Authorization Checks

The conversation deletion properly verifies ownership before deleting (backend/windmill-api/src/flow_conversations.rs:163-174)

Code Quality Issues

⚠️ Typo in Comment (backend/windmill-api/src/flow_conversations.rs:131)

// Truncate title to 25 char characters max

Should be: "Truncate title to 25 characters max"

⚠️ Missing Error Context

In memory_common.rs, deserialization errors could be more descriptive:

let messages: Vec<OpenAIMessage> = serde_json::from_value(row.messages)?;

Consider: serde_json::from_value(row.messages).map_err(|e| Error::internal_err(format!("Failed to deserialize messages: {}", e)))?

Performance Implications

1. Database Load: Storing 100KB JSONB per conversation could impact database size. Consider monitoring and potentially adding a retention policy.

2. Serialization Overhead: Converting messages to/from JSON on every read/write adds CPU overhead compared to S3.

3. No Pagination: The code reads entire message history at once. For the 100KB limit this is acceptable, but consider pagination if limits increase.

Testing Recommendations

1. Test truncation behavior with messages approaching and exceeding 100KB
2. Test concurrent writes to the same (workspace_id, conversation_id, step_id) tuple
3. Verify memory cleanup on conversation deletion
4. Test migration rollback (down.sql)
5. Test with S3 configured vs not configured

Reproduction Instructions

To test the AI agent memory functionality added in this PR:

Navigate to a flow editor in the Windmill app and add an AI agent step. Configure the agent with a provider (like OpenAI) and enable the "messages_context_length" field by setting it to a value greater than 0 (e.g., 10). This activates the conversation memory feature. Execute the flow multiple times with different user messages to build up conversation history. The AI agent should now maintain context across executions, with messages stored in the new ai_agent_memory database table (if S3 storage is not configured) or in S3 (if configured). If you're working in a workspace without S3 storage and the conversation exceeds 100KB, you should see a truncation warning in the logs suggesting S3 configuration. You can verify the memory persistence by checking that the agent's responses reference previous messages in the conversation. To test cleanup, delete a conversation from the flow conversations interface and verify the associated memory is removed from the database.

---

Summary

Strengths:

• Clean database schema design
• Good separation of OSS vs Enterprise code
• Helpful user-facing warnings about S3 storage
• Proper security and authorization checks

Areas for Improvement:

• Add database index for conversation deletion queries
• Optimize truncation algorithm for better performance
• Improve error handling for async memory deletion
• Add more context to deserialization errors

Overall Assessment: This is a solid implementation with good attention to size limits and user guidance. The main concerns are around performance optimization (truncation algorithm) and error visibility (async deletion). These are not blocking issues but should be addressed for production robustness.

[claude]

Performance: O(n²) truncation algorithm

The current truncation approach serializes the entire message array on each iteration, resulting in O(n²) complexity. For large message arrays, consider a binary search approach:

fn truncate_messages(
    messages: &[OpenAIMessage],
    max_size_bytes: usize,
) -> Result<Vec<OpenAIMessage>, Error> {
    if messages.is_empty() {
        return Ok(vec![]);
    }
    
    // Binary search for the maximum number of recent messages that fit
    let mut left = 0;
    let mut right = messages.len();
    
    while left < right {
        let mid = (left + right + 1) / 2;
        let test_messages = &messages[messages.len() - mid..];
        let test_json = serde_json::to_value(test_messages)?;
        
        if test_json.to_string().len() <= max_size_bytes {
            left = mid;
        } else {
            right = mid - 1;
        }
    }
    
    Ok(messages[messages.len() - left..].to_vec())
}

This reduces complexity from O(n²) to O(n log n).

[claude]

Suggestion: Add index for deletion queries

The delete_conversation_from_db function in memory_common.rs queries by (workspace_id, conversation_id) without the step_id. While the primary key covers the full tuple, an additional index would optimize conversation-wide deletions:

CREATE INDEX idx_ai_agent_memory_conversation 
ON ai_agent_memory (workspace_id, conversation_id);

This is especially important if conversations can have many steps, as the deletion would otherwise require a full index scan.

[claude]

Suggestion: Add error context

Deserialization errors could be more descriptive to help debugging:

let messages: Vec<OpenAIMessage> = serde_json::from_value(row.messages)
    .map_err(|e| Error::internal_err(format!("Failed to deserialize AI agent messages: {}", e)))?;

This helps identify whether corruption occurred in the database or if there's a schema mismatch.

[centdix]

/eeref

[windmill-internal-app]

Starting ee ref update...

[windmill-internal-app]

Successfully updated ee-repo-ref.txt