feat(chat): move directives into message composer#539
Conversation
Replace the header selector with a composer chip and upward-opening management popover. Persist multiple active directives per session and apply them consistently to requests and exports. Signed-off-by: abdulm6 <abdul.mohammad9087@gmail.com>
|
|
There was a problem hiding this comment.
Pull request overview
Moves chat directives from a single per-session selection in the header to a multi-select, per-session active set surfaced in the message composer (chip + upward popover), with persistence/migration and request/export application.
Changes:
- Introduces
chat_session_directives(many-to-many) and repository/service APIs for managing multiple active directives per session (including legacy compatibility projection). - Updates request transformation and exporters to apply/include all active directives.
- Reworks desktop UI state/actions to manage directives from the composer and adds localized strings + new integration tests.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/src/main/kotlin/io/askimo/core/providers/ChatRequestTransformers.kt | Applies all active session directives as system messages (deduped). |
| shared/src/main/kotlin/io/askimo/core/db/DatabaseManager.kt | Creates chat_session_directives and migrates legacy chat_sessions.directive_id. |
| shared/src/main/kotlin/io/askimo/core/chat/service/ChatSessionService.kt | Exposes active directive ID set in resume result and adds directive-set APIs. |
| shared/src/main/kotlin/io/askimo/core/chat/service/ChatSessionExporterService.kt | Exports active directive IDs (JSON/Markdown/HTML) in addition to legacy field. |
| shared/src/main/kotlin/io/askimo/core/chat/repository/ChatSessionRepository.kt | Implements persistence for session directive sets, toggling, and legacy projection syncing. |
| shared/src/main/kotlin/io/askimo/core/chat/repository/ChatDirectiveRepository.kt | Adds “find active directives by session” and refreshes session projections on delete. |
| shared/src/main/kotlin/io/askimo/core/chat/domain/ChatSession.kt | Defines ChatSessionDirectivesTable association table. |
| desktop-shared/src/main/resources/i18n/messages.properties | Adds directives chip/popover strings (en). |
| desktop-shared/src/main/resources/i18n/messages_zh_TW.properties | Adds directives chip/popover strings (zh_TW). |
| desktop-shared/src/main/resources/i18n/messages_zh_CN.properties | Adds directives chip/popover strings (zh_CN). |
| desktop-shared/src/main/resources/i18n/messages_vi_VN.properties | Adds directives chip/popover strings (vi_VN). |
| desktop-shared/src/main/resources/i18n/messages_pt_BR.properties | Adds directives chip/popover strings (pt_BR). |
| desktop-shared/src/main/resources/i18n/messages_ko_KR.properties | Adds directives chip/popover strings (ko_KR). |
| desktop-shared/src/main/resources/i18n/messages_ja_JP.properties | Adds directives chip/popover strings (ja_JP). |
| desktop-shared/src/main/resources/i18n/messages_fr.properties | Adds directives chip/popover strings (fr). |
| desktop-shared/src/main/resources/i18n/messages_es.properties | Adds directives chip/popover strings (es). |
| desktop-shared/src/main/resources/i18n/messages_de.properties | Adds directives chip/popover strings (de). |
| desktop-shared/src/main/kotlin/io/askimo/ui/session/SessionManager.kt | Persists multiple active directive IDs on first message/session creation. |
| desktop-shared/src/main/kotlin/io/askimo/ui/chat/ChatViewModel.kt | Replaces single directive state with activeDirectiveIds and toggling persistence. |
| desktop-shared/src/main/kotlin/io/askimo/ui/chat/ChatView.kt | Removes header selector and wires composer chip/popover + edit/manage flows. |
| desktop-shared/src/main/kotlin/io/askimo/ui/chat/ChatState.kt | Replaces selectedDirective with activeDirectiveIds in UI state. |
| desktop-shared/src/main/kotlin/io/askimo/ui/chat/ChatInputField.kt | Adds directives chip with active-count badge and upward popup UI. |
| desktop-shared/src/main/kotlin/io/askimo/ui/chat/ChatActions.kt | Updates actions API to toggle directives on/off. |
| cli/src/test/kotlin/io/askimo/core/providers/ChatRequestTransformersTest.kt | Adds tests for multi-directive application, ordering, and dedupe/blank filtering. |
| cli/src/test/kotlin/io/askimo/core/db/SessionDirectiveMigrationIT.kt | Adds integration test for legacy migration and non-resurrection after disable. |
| cli/src/test/kotlin/io/askimo/core/chat/service/ChatSessionServiceIT.kt | Verifies resume returns all active directives. |
| cli/src/test/kotlin/io/askimo/core/chat/repository/ChatSessionDirectiveRepositoryIT.kt | Adds repository integration tests for multi-directive persistence + sync behaviors. |
Comments suppressed due to low confidence (1)
shared/src/main/kotlin/io/askimo/core/chat/repository/ChatSessionRepository.kt:317
setSessionDirectiveActive()upserts intochat_session_directiveswithout verifying the directive exists. WithPRAGMA foreign_keys = ONand a FK tochat_directives(id), toggling a directive ID that has been deleted (or hasn’t been synced locally yet) will throw a constraint exception and can crash the caller; the method signature suggests it should return false on failure.
if (active) {
ChatSessionDirectivesTable.upsert {
it[ChatSessionDirectivesTable.sessionId] = sessionId
it[ChatSessionDirectivesTable.directiveId] = directiveId
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val activeIds = ChatSessionDirectivesTable | ||
| .selectAll() | ||
| .where { ChatSessionDirectivesTable.sessionId eq sessionId } | ||
| .mapTo(linkedSetOf()) { it[ChatSessionDirectivesTable.directiveId] } |
| /** Compatibility accessor for callers that still expect one directive. */ | ||
| val directiveId: String? | ||
| get() = activeDirectiveIds.firstOrNull() |
| onEditDirective: (ChatDirective) -> Unit, | ||
| onManageDirectives: (() -> Unit)?, | ||
| ) { | ||
| var showPopup by remember { mutableStateOf(false) } |
Order active directive IDs deterministically, keep the legacy compatibility projection stable, and reset the directives popover when switching sessions. Signed-off-by: abdulm6 <abdul.mohammad9087@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
shared/src/main/kotlin/io/askimo/core/chat/repository/ChatSessionRepository.kt:387
updateLegacyDirectiveProjection()always rewriteschat_sessions.directive_idfrom the relation table. If a session was synced from an older payload anddirective_idreferences a directive row that hasn’t been synced locally yet, activating any other directive will overwritedirective_idand permanently lose the legacy selection (so it can’t be migrated later when the directive finally exists). To preserve legacy selections, avoid overwritingchat_sessions.directive_idwhen it points to a directive that doesn’t exist locally and also isn’t yet present inchat_session_directives.
private fun updateLegacyDirectiveProjection(sessionId: String) {
val legacyDirectiveId = ChatSessionDirectivesTable
.selectAll()
.where { ChatSessionDirectivesTable.sessionId eq sessionId }
.orderBy(ChatSessionDirectivesTable.directiveId, SortOrder.ASC)
.limit(1)
.singleOrNull()
?.get(ChatSessionDirectivesTable.directiveId)
ChatSessionsTable.update({ ChatSessionsTable.id eq sessionId }) {
it[directiveId] = legacyDirectiveId
it[updatedAt] = Instant.now()
}
}
| private fun populateRelationFromLegacyIfEmpty(sessionId: String) { | ||
| val hasRelations = ChatSessionDirectivesTable | ||
| .selectAll() | ||
| .where { ChatSessionDirectivesTable.sessionId eq sessionId } | ||
| .any() | ||
| if (hasRelations) return | ||
|
|
||
| val legacyDirectiveId = ChatSessionsTable | ||
| .selectAll() | ||
| .where { ChatSessionsTable.id eq sessionId } | ||
| .singleOrNull() | ||
| ?.get(ChatSessionsTable.directiveId) | ||
| ?: return | ||
| val directiveExists = ChatDirectivesTable | ||
| .selectAll() | ||
| .where { ChatDirectivesTable.id eq legacyDirectiveId } | ||
| .any() | ||
| if (directiveExists) { | ||
| ChatSessionDirectivesTable.insert { | ||
| it[ChatSessionDirectivesTable.sessionId] = sessionId | ||
| it[ChatSessionDirectivesTable.directiveId] = legacyDirectiveId | ||
| } | ||
| } | ||
| } |
Preserve unresolved legacy directive selections while other directives are toggled, then restore the missing relation when its directive arrives from sync. Add regression coverage for the session-before-directive ordering. Signed-off-by: abdulm6 <abdul.mohammad9087@gmail.com>
| Checkbox( | ||
| checked = isActive, | ||
| onCheckedChange = null, | ||
| modifier = Modifier.pointerHoverIcon(PointerIcon.Hand), | ||
| ) |
| appendLine(" \"createdAt\": \"${session.createdAt.atOffset(ZoneOffset.UTC).format(timestampFormatter)}\",") | ||
| appendLine(" \"lastUpdated\": \"${session.updatedAt.atOffset(java.time.ZoneOffset.UTC).format(timestampFormatter)}\",") | ||
| appendLine(" \"directiveId\": ${if (session.directiveId != null) "\"${escapeJson(session.directiveId)}\"" else "null"},") | ||
| appendLine(" \"directiveIds\": [$directiveIdsJson],") |
Expose each directive row as one accessible checkbox toggle and derive the JSON compatibility directive from the sorted active set. Add integration coverage for unresolved legacy selections during export. Signed-off-by: abdulm6 <abdul.mohammad9087@gmail.com>
Summary
Motivation
Directives affect the next message, so placing their control beside the message input makes their purpose clearer and keeps the session header compact. The chip and popover also scale cleanly when a session has many directives.
How to test
Verification
./gradlew spotlessApply./gradlew spotlessCheck./gradlew detekt./gradlew cleanTest build --no-build-cacheCloses #535