Skip to content

[codex] Add AI conversation store coverage - #7722

Open
sfmskywalker wants to merge 5 commits into
mainfrom
codex/increase-ai-conversation-store-coverage
Open

[codex] Add AI conversation store coverage#7722
sfmskywalker wants to merge 5 commits into
mainfrom
codex/increase-ai-conversation-store-coverage

Conversation

@sfmskywalker

Copy link
Copy Markdown
Member

Summary

Adds focused unit coverage for InMemoryAIConversationStore in the AI host module.

The new tests cover:

  • required conversation ID and user ID validation
  • tenant and user ownership checks when overwriting conversations
  • same-owner overwrite behavior
  • ephemeral, durable, and configured retention behavior
  • pruning expired conversations during saves

Validation

  • dotnet test test/unit/Elsa.AI.Host.UnitTests/Elsa.AI.Host.UnitTests.csproj

Coverlet reports Elsa.AI.Host at 30.75% line coverage for the targeted test project after this change.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds focused unit tests for InMemoryAIConversationStore in the AI host module to increase confidence around validation, ownership rules, and retention/expiry behavior.

Changes:

  • Added unit coverage for required fields (conversation ID, user ID) when saving.
  • Added tests for tenant/user ownership enforcement during overwrite scenarios (including same-owner overwrite).
  • Added tests for retention behavior (ephemeral, durable, configured deadlines) and pruning of expired conversations during saves.

Comment thread test/unit/Elsa.AI.Host.UnitTests/InMemoryAIConversationStoreTests.cs Outdated
@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a bug where AIConversationStatus.Expired was not treated as a terminal state for ephemeral retention, causing conversations in that status to linger instead of being evicted. The fix is applied consistently to both InMemoryAIConversationStore and the EFCore path. Existing tests in AIRegistrationTests are migrated to a dedicated InMemoryAIConversationStoreTests class with expanded, focused coverage, and EFCoreAIConversationStoreTests is updated to exercise the new terminal state. The CShells package suite is promoted from 0.0.24-preview.132 to the stable 0.0.24 release and its NuGet source mapping is updated accordingly.

  • Bug fix: IsExpired in both InMemoryAIConversationStore and EFCoreAIConversationStore (and the EFCore cleanup service) now includes AIConversationStatus.Expired in the ephemeral terminal-state check, making all three implementations consistent.
  • Test migration and expansion: Tests previously scattered in AIRegistrationTests are moved to a purpose-built file; new cases cover the Expired status, Configured-mode null-deadline retention, pruning survival of active records, and same-owner overwrites.
  • Dependency update: CShells packages upgraded to stable 0.0.24, removing the feedz.io preview dependency and adding explicit NuGet.org source mapping entries.

Confidence Score: 5/5

Safe to merge — the changes are a targeted bug fix with consistent coverage across both store implementations and their tests.

The diff adds AIConversationStatus.Expired to the ephemeral terminal-state check in three places (in-memory store, EFCore store, EFCore cleanup service) and backs each change with matching tests. The logic is straightforward, the test suite is thorough, and the package update is a stable-release promotion with no API-breaking implications.

No files require special attention.

Important Files Changed

Filename Overview
src/modules/Elsa.AI.Host/Services/InMemoryAIConversationStore.cs One-line bug fix: adds AIConversationStatus.Expired to the ephemeral terminal-state check in IsExpired, consistent with EFCore counterpart.
src/modules/Elsa.AI.Persistence.EFCore/Services/EFCoreAIConversationCleanup.cs Adds expiredStatus variable and includes it in the ephemeral-retention delete filter, mirroring the in-memory fix.
src/modules/Elsa.AI.Persistence.EFCore/Stores/EFCoreAIConversationStore.cs Same single-line fix as the in-memory store — AIConversationStatus.Expired added to IsExpired for ephemeral mode.
test/unit/Elsa.AI.Host.UnitTests/InMemoryAIConversationStoreTests.cs New dedicated test class with 11 focused scenarios; covers validation, ownership, all retention modes, the new Expired terminal state, pruning survival, and case-insensitive ID matching.
test/unit/Elsa.AI.Host.UnitTests/AIRegistrationTests.cs Removes in-memory store tests that were relocated to InMemoryAIConversationStoreTests; remaining registration tests are untouched.
test/unit/Elsa.AI.Persistence.EFCore.UnitTests/EFCoreAIConversationStoreTests.cs Converts the single-status ephemeral-hide fact to a three-status theory and adds an Expired ephemeral entry to the cleanup test, updating the expected deletion count to 3.
Directory.Packages.props Promotes six CShells packages from 0.0.24-preview.132 to the stable 0.0.24 release.
NuGet.Config Adds explicit CShells and CShells.* patterns to the NuGet official source mapping so the stable release resolves from nuget.org.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[IsExpired called] --> B{RetentionMode?}
    B -->|Ephemeral| C{Status?}
    C -->|Completed| D[✅ Expired → evict]
    C -->|Failed| D
    C -->|Expired NEW| D
    C -->|Active| E[❌ Not expired → keep]
    B -->|Durable| F[❌ Not expired → keep]
    B -->|Configured| G{RetentionExpiresAt?}
    G -->|null| H[❌ Not expired → keep]
    G -->|past deadline| I[✅ Expired → evict]
    G -->|future deadline| H
Loading

Reviews (5): Last reviewed commit: "Fix CShells package restore source" | Re-trigger Greptile

Comment thread test/unit/Elsa.AI.Host.UnitTests/InMemoryAIConversationStoreTests.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread src/modules/Elsa.AI.Host/Services/InMemoryAIConversationStore.cs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread Directory.Packages.props
Comment on lines +109 to +114
<PackageVersion Include="CShells" Version="0.0.24"/>
<PackageVersion Include="CShells.Abstractions" Version="0.0.24"/>
<PackageVersion Include="CShells.AspNetCore" Version="0.0.24"/>
<PackageVersion Include="CShells.AspNetCore.Abstractions" Version="0.0.24"/>
<PackageVersion Include="CShells.FastEndpoints" Version="0.0.24"/>
<PackageVersion Include="CShells.FastEndpoints.Abstractions" Version="0.0.24"/>
Comment thread NuGet.Config
Comment on lines 12 to 16
<packageSource key="NuGet official package source">
<package pattern="*" />
<package pattern="CShells" />
<package pattern="CShells.*" />
</packageSource>
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