feat: Assistant chat database tables and repository#18011
Conversation
…d as jsonb lists on ChatMessage
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a chat subsystem to Designer: new domain and EF Core DB models, enums, mappers, repository interface and implementation, entity configurations, EF migration creating chat tables, and DI registration; plus a short documentation guideline update. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/Controller
participant Repo as ChatRepository
participant Mapper as ChatThreadMapper
participant DbContext as DesignerdbContext
participant DB as Database
rect rgba(0, 100, 200, 0.5)
Note over Client,DB: Create Thread
Client->>Repo: CreateThreadAsync(ChatThreadEntity)
Repo->>Mapper: MapToDbModel(thread)
Mapper-->>Repo: ChatThreadDbModel
Repo->>DbContext: ChatThreads.Add(dbModel)
Repo->>DbContext: SaveChangesAsync()
DbContext->>DB: INSERT designer.chat_threads
DB-->>DbContext: OK
Repo->>Mapper: MapToModel(dbModel)
Mapper-->>Repo: ChatThreadEntity (with Id)
Repo-->>Client: Created ChatThreadEntity
end
rect rgba(0, 150, 100, 0.5)
Note over Client,DB: Get Threads
Client->>Repo: GetThreadsAsync(AltinnRepoEditingContext)
Repo->>DbContext: Query ChatThreads (Org,App,CreatedBy)
DbContext->>DB: SELECT ... ORDER BY CreatedAt DESC
DB-->>DbContext: List<ChatThreadDbModel>
Repo->>Mapper: MapToModel(each)
Mapper-->>Repo: List<ChatThreadEntity>
Repo-->>Client: List<ChatThreadEntity>
end
rect rgba(200, 100, 0, 0.5)
Note over Client,DB: Create Message
Client->>Repo: CreateMessageAsync(threadId, ChatMessageEntity)
Repo->>ChatMessageMapper: MapToDbModel(message)
ChatMessageMapper-->>Repo: ChatMessageDbModel (set ThreadId)
Repo->>DbContext: ChatMessages.Add(dbModel)
Repo->>DbContext: SaveChangesAsync()
DbContext->>DB: INSERT designer.chat_messages
DB-->>DbContext: OK
Repo->>ChatMessageMapper: MapToModel(dbModel)
ChatMessageMapper-->>Repo: ChatMessageEntity (with Id)
Repo-->>Client: Created ChatMessageEntity
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable sequence diagrams in the walkthrough.Disable the |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
src/Designer/backend/AGENTS.md (1)
68-70: Helpful addition to establish consistent patterns.The new coding guideline clearly documents the parameter pattern for the repository layer, which aligns well with the chat repository implementation being introduced in this PR.
Optional: Consider adding scope and an example for clarity
To help future developers understand where and how to apply this guideline, you might optionally:
- Specify the scope more explicitly (e.g., "in repository method signatures")
- Add a brief example showing the pattern
Example enhancement:
## Coding guidelines -- Use AltinnRepoEditingContext instead of `org`, `repo`/`app` and `developer`/`user` as individual parameters. +- Use AltinnRepoEditingContext instead of `org`, `repo`/`app` and `developer`/`user` as individual parameters in repository method signatures. + ```csharp + // Preferred + Task<ChatThread> CreateThreadAsync(AltinnRepoEditingContext context, ...) + + // Avoid + Task<ChatThread> CreateThreadAsync(string org, string app, string developer, ...) + ```However, the current guideline is clear and functional as-is.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Designer/backend/AGENTS.md` around lines 68 - 70, The guideline should clarify the scope and give an example using AltinnRepoEditingContext: update the sentence under "Coding guidelines" to specify "in repository method signatures" and add a short example showing the preferred versus avoided signatures (reference symbols like AltinnRepoEditingContext and example method names such as CreateThreadAsync or CreateChatRepositoryAsync) so readers see usage context; keep the existing line but append one or two example lines demonstrating Task<ChatThread> CreateThreadAsync(AltinnRepoEditingContext context, ...) as preferred and Task<ChatThread> CreateThreadAsync(string org, string app, string developer, ...) as avoided.src/Designer/backend/src/Designer/Repository/Models/ChatMessageEntity.cs (1)
17-28: Prefer chat-specific enum names for clearer domain boundaries.
RoleandActionModeare quite generic. Renaming to domain-specific names (for exampleChatRole/ChatActionMode) will improve discoverability and reduce ambiguous imports.As per coding guidelines: "Use meaningful and descriptive names, and avoid abbreviations".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Designer/backend/src/Designer/Repository/Models/ChatMessageEntity.cs` around lines 17 - 28, Rename the generic enums Role and ActionMode to domain-specific names (e.g., ChatRole and ChatActionMode) inside ChatMessageEntity.cs and update all usages across the codebase to the new names; specifically change the enum declarations (Role -> ChatRole, ActionMode -> ChatActionMode) and then refactor all references (type annotations, comparisons, switch cases, JSON (de)serialization attributes, and any mappings) to use ChatRole and ChatActionMode so compilation and runtime behavior remain identical. Ensure any serialization names or database mappings are preserved (update attributes if they reference the old enum names) and run tests/CI to catch missed references.src/Designer/backend/src/Designer/Repository/ORMImplementation/Data/EntityConfigurations/ChatThreadConfiguration.cs (2)
11-35: Extract schema/table/column identifiers to constants.This block contains many repeated string literals. Centralising them will reduce typo risk and simplify future renames.
As per coding guidelines: "Avoid hard-coded numbers and strings".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Designer/backend/src/Designer/Repository/ORMImplementation/Data/EntityConfigurations/ChatThreadConfiguration.cs` around lines 11 - 35, The configuration uses repeated string literals for schema, table and column names; define and use constants (e.g., private const string Schema = "designer"; private const string Table = "chat_threads"; and column constants like IdColumn="id", OrgColumn="org", AppColumn="app", CreatedByColumn="created_by", TitleColumn="title", CreatedAtColumn="created_at") at the top of the ChatThreadConfiguration class and replace occurrences in builder.ToTable, builder.Property(...).HasColumnName(...) and the HasIndex name to reference those constants (also build the index name from constants or a single IndexName constant) so all identifiers are centralized and typos/renames are avoided, leaving builder.HasKey(e => e.Id) and property mappings otherwise unchanged.
15-23: IncludeCreatedAtin the thread lookup index to match query ordering.Current reads filter by org/app/created_by and then sort by
created_at. Extending the index withcreated_atwill reduce sort cost as data grows.Proposed index update
- builder.HasIndex( - e => new - { - e.Org, - e.App, - e.CreatedBy, - }, - "idx_chat_threads_org_app_created_by" - ); + builder.HasIndex( + e => new + { + e.Org, + e.App, + e.CreatedBy, + e.CreatedAt, + }, + "idx_chat_threads_org_app_created_by_created_at" + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Designer/backend/src/Designer/Repository/ORMImplementation/Data/EntityConfigurations/ChatThreadConfiguration.cs` around lines 15 - 23, The index defined in builder.HasIndex currently covers e.Org, e.App, e.CreatedBy but queries also order by e.CreatedAt; update the index to include e.CreatedAt in the anonymous key passed to builder.HasIndex (and optionally rename the index "idx_chat_threads_org_app_created_by" to reflect created_at) so the DB can use the index for the sort on CreatedAt and avoid an extra sort step.src/Designer/backend/src/Designer/Repository/ORMImplementation/Data/EntityConfigurations/ChatMessageConfiguration.cs (2)
17-17: Add a composite index for message reads ordered by time.Message reads are filtered by
thread_idand ordered bycreated_at. A(thread_id, created_at)index will better match that access pattern.Proposed index update
- builder.HasIndex(e => e.ThreadId, "idx_chat_messages_thread_id"); + builder.HasIndex( + e => new + { + e.ThreadId, + e.CreatedAt, + }, + "idx_chat_messages_thread_id_created_at" + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Designer/backend/src/Designer/Repository/ORMImplementation/Data/EntityConfigurations/ChatMessageConfiguration.cs` at line 17, The current single-column index builder.HasIndex(e => e.ThreadId, "idx_chat_messages_thread_id") should be extended to include created time so queries filtering by ThreadId and ordering by CreatedAt use a composite index; update the configuration to add a composite index using builder.HasIndex(e => new { e.ThreadId, e.CreatedAt }).HasDatabaseName("idx_chat_messages_thread_id_created_at") (or replace the existing ThreadId-only index with this composite index) referencing the ChatMessage entity properties ThreadId and CreatedAt.
11-33: Extract mapping identifiers to constants to reduce string literal drift.Table, schema, index, and column names are all hard-coded in-place. Pulling them into constants improves maintainability.
As per coding guidelines: "Avoid hard-coded numbers and strings".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Designer/backend/src/Designer/Repository/ORMImplementation/Data/EntityConfigurations/ChatMessageConfiguration.cs` around lines 11 - 33, The mapping uses many hard-coded strings in ChatMessageConfiguration (builder.ToTable, HasIndex, and builder.Property calls); extract those identifiers into class-level private const string fields (e.g., TableName, SchemaName, Index_ThreadId, Column_Id, Column_ThreadId, Column_CreatedAt, Column_Role, Column_Content, Column_ActionMode, Column_FilesChanged, Column_AttachmentFileNames) and replace the literal usages in ToTable("chat_messages","designer"), HasIndex(...,"idx_chat_messages_thread_id"), and each Property(...).HasColumnName(...) so all table, schema, index and column names are referenced via those constants to avoid string literal drift.src/Designer/backend/src/Designer/Repository/ORMImplementation/Mappers/ChatMessageMapper.cs (1)
17-18: Avoid sharing mutable list instances across mapper boundaries.
FilesChangedandAttachmentFileNamesare copied by reference in both directions. This can leak mutations between domain and DB models.Proposed change
public static ChatMessageDbModel MapToDbModel(ChatMessageEntity entity) { return new ChatMessageDbModel { Id = entity.Id, CreatedAt = entity.CreatedAt, Role = entity.Role, Content = entity.Content, ActionMode = entity.ActionMode, - FilesChanged = entity.FilesChanged, - AttachmentFileNames = entity.AttachmentFileNames, + FilesChanged = [.. entity.FilesChanged], + AttachmentFileNames = [.. entity.AttachmentFileNames], }; } @@ public static ChatMessageEntity MapToModel(ChatMessageDbModel dbModel) { return new ChatMessageEntity { Id = dbModel.Id, CreatedAt = dbModel.CreatedAt, Role = dbModel.Role, Content = dbModel.Content, ActionMode = dbModel.ActionMode, - FilesChanged = dbModel.FilesChanged, - AttachmentFileNames = dbModel.AttachmentFileNames, + FilesChanged = [.. dbModel.FilesChanged], + AttachmentFileNames = [.. dbModel.AttachmentFileNames], }; }Also applies to: 31-32
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Designer/backend/src/Designer/Repository/ORMImplementation/Mappers/ChatMessageMapper.cs` around lines 17 - 18, The mapper is assigning FilesChanged and AttachmentFileNames by reference, which allows shared mutable lists to leak between DB and domain objects; update ChatMessageMapper to copy these collections when mapping in both directions (entity -> domain and domain -> entity) by creating new lists (e.g., new List<T>(...)/Enumerable.ToList()) and handle nulls to avoid NREs; ensure the same fix is applied for the other occurrences referenced (the properties at the other mapping block for the same names).src/Designer/backend/src/Designer/Migrations/20260302113847_ChatTables.cs (1)
61-73: Add sort-aware indexes for the chat read queries.Both chat read paths sort on
created_at; current indexes only cover filter columns, so PostgreSQL will still need extra sorting work on larger datasets.Suggested migration adjustment
- migrationBuilder.CreateIndex( - name: "idx_chat_messages_thread_id", - schema: "designer", - table: "chat_messages", - column: "thread_id" - ); + migrationBuilder.CreateIndex( + name: "idx_chat_messages_thread_id_created_at", + schema: "designer", + table: "chat_messages", + columns: new[] { "thread_id", "created_at" } + ); - migrationBuilder.CreateIndex( - name: "idx_chat_threads_org_app_created_by", - schema: "designer", - table: "chat_threads", - columns: new[] { "org", "app", "created_by" } - ); + migrationBuilder.CreateIndex( + name: "idx_chat_threads_org_app_created_by_created_at", + schema: "designer", + table: "chat_threads", + columns: new[] { "org", "app", "created_by", "created_at" } + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Designer/backend/src/Designer/Migrations/20260302113847_ChatTables.cs` around lines 61 - 73, The current indexes created via migrationBuilder.CreateIndex (idx_chat_messages_thread_id and idx_chat_threads_org_app_created_by) only include filter columns and do not cover the queries' ORDER BY on created_at, causing PostgreSQL to perform extra sorts; update those CreateIndex calls to include "created_at" as the final column (e.g., add column: "created_at" to idx_chat_messages_thread_id and append "created_at" to the columns array for idx_chat_threads_org_app_created_by) so the indexes are sort-aware and can support index-only ordering for chat read queries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Designer/backend/src/Designer/Repository/IChatRepository.cs`:
- Around line 38-65: The methods UpdateThreadAsync, DeleteThreadAsync,
GetMessagesAsync, and CreateMessageAsync must accept an AltinnRepoEditingContext
so repository implementations can enforce per-thread authorization; update the
interface signatures for Task UpdateThreadAsync(ChatThreadEntity thread,
CancellationToken cancellationToken = default), Task DeleteThreadAsync(Guid id,
CancellationToken cancellationToken = default), Task<List<ChatMessageEntity>>
GetMessagesAsync(Guid threadId, CancellationToken cancellationToken = default),
and Task<ChatMessageEntity> CreateMessageAsync(Guid threadId, ChatMessageEntity
message, CancellationToken cancellationToken = default) to add an
AltinnRepoEditingContext parameter (e.g., AltinnRepoEditingContext
editingContext) and propagate that parameter through implementations so
ownership can be validated (org/app/created_by) before performing thread-scoped
operations.
In
`@src/Designer/backend/src/Designer/Repository/ORMImplementation/ChatRepository.cs`:
- Around line 86-90: Before inserting, verify the thread exists: in
ChatRepository (CreateMessageAsync) call _dbContext.Threads.AnyAsync(t => t.Id
== threadId, cancellationToken) (or the equivalent threads set) and if it
returns false, short-circuit with a controlled not-found error (throw the
project's NotFoundException type or a KeyNotFoundException) instead of letting
the DB raise. Do this check before mapping/adding the message (before
ChatMessageMapper.MapToDbModel and _dbContext.ChatMessages.Add) and pass the
cancellationToken into the async query.
In
`@src/Designer/backend/src/Designer/Repository/ORMImplementation/Data/EntityConfigurations/ChatMessageConfiguration.cs`:
- Around line 31-33: The JSONB properties configured with builder.Property(e =>
e.FilesChanged).HasColumnType("jsonb").HasColumnName("files_changed") and
builder.Property(e =>
e.AttachmentFileNames).HasColumnType("jsonb").HasColumnName("attachment_file_names")
are missing the model-level non-null constraint; update both property
configurations to include .IsRequired() so the EF model matches the
migration/schema and keeps consistency with other properties.
---
Nitpick comments:
In `@src/Designer/backend/AGENTS.md`:
- Around line 68-70: The guideline should clarify the scope and give an example
using AltinnRepoEditingContext: update the sentence under "Coding guidelines" to
specify "in repository method signatures" and add a short example showing the
preferred versus avoided signatures (reference symbols like
AltinnRepoEditingContext and example method names such as CreateThreadAsync or
CreateChatRepositoryAsync) so readers see usage context; keep the existing line
but append one or two example lines demonstrating Task<ChatThread>
CreateThreadAsync(AltinnRepoEditingContext context, ...) as preferred and
Task<ChatThread> CreateThreadAsync(string org, string app, string developer,
...) as avoided.
In `@src/Designer/backend/src/Designer/Migrations/20260302113847_ChatTables.cs`:
- Around line 61-73: The current indexes created via
migrationBuilder.CreateIndex (idx_chat_messages_thread_id and
idx_chat_threads_org_app_created_by) only include filter columns and do not
cover the queries' ORDER BY on created_at, causing PostgreSQL to perform extra
sorts; update those CreateIndex calls to include "created_at" as the final
column (e.g., add column: "created_at" to idx_chat_messages_thread_id and append
"created_at" to the columns array for idx_chat_threads_org_app_created_by) so
the indexes are sort-aware and can support index-only ordering for chat read
queries.
In `@src/Designer/backend/src/Designer/Repository/Models/ChatMessageEntity.cs`:
- Around line 17-28: Rename the generic enums Role and ActionMode to
domain-specific names (e.g., ChatRole and ChatActionMode) inside
ChatMessageEntity.cs and update all usages across the codebase to the new names;
specifically change the enum declarations (Role -> ChatRole, ActionMode ->
ChatActionMode) and then refactor all references (type annotations, comparisons,
switch cases, JSON (de)serialization attributes, and any mappings) to use
ChatRole and ChatActionMode so compilation and runtime behavior remain
identical. Ensure any serialization names or database mappings are preserved
(update attributes if they reference the old enum names) and run tests/CI to
catch missed references.
In
`@src/Designer/backend/src/Designer/Repository/ORMImplementation/Data/EntityConfigurations/ChatMessageConfiguration.cs`:
- Line 17: The current single-column index builder.HasIndex(e => e.ThreadId,
"idx_chat_messages_thread_id") should be extended to include created time so
queries filtering by ThreadId and ordering by CreatedAt use a composite index;
update the configuration to add a composite index using builder.HasIndex(e =>
new { e.ThreadId, e.CreatedAt
}).HasDatabaseName("idx_chat_messages_thread_id_created_at") (or replace the
existing ThreadId-only index with this composite index) referencing the
ChatMessage entity properties ThreadId and CreatedAt.
- Around line 11-33: The mapping uses many hard-coded strings in
ChatMessageConfiguration (builder.ToTable, HasIndex, and builder.Property
calls); extract those identifiers into class-level private const string fields
(e.g., TableName, SchemaName, Index_ThreadId, Column_Id, Column_ThreadId,
Column_CreatedAt, Column_Role, Column_Content, Column_ActionMode,
Column_FilesChanged, Column_AttachmentFileNames) and replace the literal usages
in ToTable("chat_messages","designer"),
HasIndex(...,"idx_chat_messages_thread_id"), and each
Property(...).HasColumnName(...) so all table, schema, index and column names
are referenced via those constants to avoid string literal drift.
In
`@src/Designer/backend/src/Designer/Repository/ORMImplementation/Data/EntityConfigurations/ChatThreadConfiguration.cs`:
- Around line 11-35: The configuration uses repeated string literals for schema,
table and column names; define and use constants (e.g., private const string
Schema = "designer"; private const string Table = "chat_threads"; and column
constants like IdColumn="id", OrgColumn="org", AppColumn="app",
CreatedByColumn="created_by", TitleColumn="title", CreatedAtColumn="created_at")
at the top of the ChatThreadConfiguration class and replace occurrences in
builder.ToTable, builder.Property(...).HasColumnName(...) and the HasIndex name
to reference those constants (also build the index name from constants or a
single IndexName constant) so all identifiers are centralized and typos/renames
are avoided, leaving builder.HasKey(e => e.Id) and property mappings otherwise
unchanged.
- Around line 15-23: The index defined in builder.HasIndex currently covers
e.Org, e.App, e.CreatedBy but queries also order by e.CreatedAt; update the
index to include e.CreatedAt in the anonymous key passed to builder.HasIndex
(and optionally rename the index "idx_chat_threads_org_app_created_by" to
reflect created_at) so the DB can use the index for the sort on CreatedAt and
avoid an extra sort step.
In
`@src/Designer/backend/src/Designer/Repository/ORMImplementation/Mappers/ChatMessageMapper.cs`:
- Around line 17-18: The mapper is assigning FilesChanged and
AttachmentFileNames by reference, which allows shared mutable lists to leak
between DB and domain objects; update ChatMessageMapper to copy these
collections when mapping in both directions (entity -> domain and domain ->
entity) by creating new lists (e.g., new List<T>(...)/Enumerable.ToList()) and
handle nulls to avoid NREs; ensure the same fix is applied for the other
occurrences referenced (the properties at the other mapping block for the same
names).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/Designer/backend/AGENTS.mdsrc/Designer/backend/src/Designer/Infrastructure/ServiceRegistration.cssrc/Designer/backend/src/Designer/Migrations/20260302113847_ChatTables.Designer.cssrc/Designer/backend/src/Designer/Migrations/20260302113847_ChatTables.cssrc/Designer/backend/src/Designer/Migrations/DesignerdbContextModelSnapshot.cssrc/Designer/backend/src/Designer/Repository/IChatRepository.cssrc/Designer/backend/src/Designer/Repository/Models/ChatMessageEntity.cssrc/Designer/backend/src/Designer/Repository/Models/ChatThreadEntity.cssrc/Designer/backend/src/Designer/Repository/ORMImplementation/ChatRepository.cssrc/Designer/backend/src/Designer/Repository/ORMImplementation/Data/DesignerdbContext.cssrc/Designer/backend/src/Designer/Repository/ORMImplementation/Data/EntityConfigurations/ChatMessageConfiguration.cssrc/Designer/backend/src/Designer/Repository/ORMImplementation/Data/EntityConfigurations/ChatThreadConfiguration.cssrc/Designer/backend/src/Designer/Repository/ORMImplementation/Mappers/ChatMessageMapper.cssrc/Designer/backend/src/Designer/Repository/ORMImplementation/Mappers/ChatThreadMapper.cssrc/Designer/backend/src/Designer/Repository/ORMImplementation/Models/ChatMessageDbModel.cssrc/Designer/backend/src/Designer/Repository/ORMImplementation/Models/ChatThreadDbModel.cs
Konrad-Simso
left a comment
There was a problem hiding this comment.
Very nice start, i've found some things that could use another look and maybe chats and threads should have their own repositories?
Description
This PR adds new tables
chat_threadsandchat_messagesinstudio-db.PR includes
PR excludes
ER diagram
erDiagram ChatThread { uuid Id PK text Org text App text CreatedBy timestamp CreatedAt text Title } ChatMessage { uuid Id PK uuid ThreadId FK timestamp CreatedAt int Role text Content int ActionMode "nullable" text[] AttachmentFileNames "nullable" text[] FilesChanged "nullable" } ChatThread ||--o{ ChatMessage : "has"Verification
Summary by CodeRabbit
New Features
Documentation