Skip to content

feat(assistant): give a conversation turn an explicit identity - #318

Merged
kl3inIT merged 1 commit into
mainfrom
feat/assistant-turn-identity
Aug 6, 2026
Merged

feat(assistant): give a conversation turn an explicit identity#318
kl3inIT merged 1 commit into
mainfrom
feat/assistant-turn-identity

Conversation

@kl3inIT

@kl3inIT kl3inIT commented Aug 6, 2026

Copy link
Copy Markdown
Owner

First of two PRs collapsing the Assistant conversation to one persisted store
(docs/increments/active/2026-08-06-assistant-conversation-memory-ssot). This
one only establishes turn identity; nothing reads it yet.

Why

beginTurn persists the question and completeTurn persists the answer, in
separate transactions. Two turns of one conversation can open before either
answers, so the rows can land as U1, U2, A2, A1. No ordering heuristic over
sequence_id pairs those correctly.

The read-only transcript context advisor that replaces MessageChatMemoryAdvisor
in the next PR has to pair them. Recording the pairing the writers already know
is cheaper and correct; inferring it later is neither. Shipping the schema first
means a wrong advisor never forces a migration rollback.

What

  • V26 adds turn_id plus a partial unique index on (turn_id, role) — one
    question and one answer per turn.
  • beginTurn allocates the turn id and returns AssistantTurnRef;
    completeTurn takes that reference instead of a bare conversation id.
  • Rows written before the migration keep a null turn_id. They stay visible in
    the transcript and are exempt from the index: their pairing cannot be
    recovered after the fact, and the debate's binding constraint is that legacy
    rows are transcript-visible but context-ineligible.

Verification

:core:test and :apps:api:test green. New AssistantTurnIdentityIntegrationTests
covers pairing, the out-of-order U1,U2,A2,A1 interleaving, 12 concurrent
beginTurn calls in one conversation, the unique-index rejection, and legacy
null rows coexisting.

Found, not fixed

Two turns of one conversation completing at the same instant raise an
optimistic-locking failure: completeTurn touches the conversation row without
a lock while beginTurn takes a pessimistic one. Pre-existing on main and
unrelated to turn identity — recorded as a gap in the Assistant test matrix.

skip-release: internal turn identity and schema only; no user-visible behavior changes in this PR

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Assistant conversation turns now have unique identities, improving reliable pairing of user questions and assistant responses.
    • Concurrent and overlapping turns are tracked independently, including when responses complete out of order.
    • Existing conversation messages remain supported.
  • Bug Fixes

    • Prevented duplicate user or assistant messages from being associated with the same turn.
  • Tests

    • Added coverage for concurrent turns, message pairing, duplicate prevention, and legacy message compatibility.

A turn writes its question in beginTurn and its answer in completeTurn, in
separate transactions. Two turns of one conversation can therefore persist as
U1, U2, A2, A1, and no ordering heuristic over sequence_id pairs them. Nothing
reads the pairing today, but the transcript context reader that replaces Spring
AI's chat memory must, and it cannot recover from sequence order what the
writers already knew.

beginTurn now allocates a turn id and returns it with the conversation id;
completeTurn takes that reference and writes the answer under the same
identity. A partial unique index over (turn_id, role) holds one question and
one answer per turn. Rows written before this migration keep a null turn_id:
they stay visible in the transcript and are exempt from the index, because
their pairing cannot be recovered after the fact.

Two turns completing at the same instant still raise an optimistic-locking
failure through the unlocked conversation touch in completeTurn. That is
pre-existing and unrelated to turn identity; it is recorded as a gap in the
Assistant test matrix rather than fixed here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f0ce53e7-c2a4-4823-95fb-e034e378b7e8

📥 Commits

Reviewing files that changed from the base of the PR and between e13685e and 7be87bf.

⛔ Files ignored due to path filters (3)
  • docs/increments/active/2026-08-06-assistant-conversation-memory-ssot/plan.md is excluded by !docs/**
  • docs/specs/domains/assistant-and-mcp.md is excluded by !docs/**
  • docs/tests/domains/assistant-and-mcp.md is excluded by !docs/**
📒 Files selected for processing (10)
  • apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantAnswerFeedbackConcurrencyIntegrationTests.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantControllerStreamingTests.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantModelSelectionConcurrencyIntegrationTests.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantTurnIdentityIntegrationTests.java
  • core/src/main/java/com/orgmemory/core/assistant/AssistantConversationMessage.java
  • core/src/main/java/com/orgmemory/core/assistant/AssistantConversationService.java
  • core/src/main/java/com/orgmemory/core/assistant/AssistantTurnRef.java
  • core/src/main/resources/db/migration/V26__assistant_message_turn_identity.sql
  • core/src/test/java/com/orgmemory/core/assistant/AssistantConversationServiceTests.java
📜 Recent review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: Backend · Java 25
  • GitHub Check: PostgreSQL GraphRAG
  • GitHub Check: Web · Node 24
🧰 Additional context used
📓 Path-based instructions (4)
**/*

📄 CodeRabbit inference engine (AGENTS.md)

**/*: Treat the repository and runtime evidence as the engineering system of record; do not treat chat or Northstar as authoritative.
Before changing a domain, read its specification, test-coverage document, and applicable decision filenames.
Material decisions about domain boundaries, authorization, persistence, publication, concurrency, cache isolation, parity scope, or deployment require an independent architecture challenge and documented alternatives before implementation.
Do not use completed increment documents as the source for current behavior; use them only for history or archaeology.
Before using unfamiliar Spring Boot, Spring Modulith, Spring AI, Gradle, React, Vite, Tailwind, TypeScript, Next.js, or Fumadocs APIs, consult current official documentation, Context7, and the relevant verification skill.
Read docs/guidelines/agent-safety.md before retrieval, AI, MCP, permission, upload, graph, or export work; never commit secrets or customer data.
Keep ddl-auto=validate and pair every persisted-model change with a Flyway migration.
Use the testing harness; a terminating clean test is the JVM context gate, and bootRun is not verification.

Files:

  • core/src/main/resources/db/migration/V26__assistant_message_turn_identity.sql
  • core/src/main/java/com/orgmemory/core/assistant/AssistantTurnRef.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantControllerStreamingTests.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantModelSelectionConcurrencyIntegrationTests.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantAnswerFeedbackConcurrencyIntegrationTests.java
  • core/src/main/java/com/orgmemory/core/assistant/AssistantConversationMessage.java
  • apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java
  • core/src/main/java/com/orgmemory/core/assistant/AssistantConversationService.java
  • core/src/test/java/com/orgmemory/core/assistant/AssistantConversationServiceTests.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantTurnIdentityIntegrationTests.java
core/src/main/resources/db/migration/*.sql

⚙️ CodeRabbit configuration file

core/src/main/resources/db/migration/*.sql: The repository is pre-release: V1 is the intentionally resettable clean
baseline and development data carries no migration cost. Once a release
baseline is frozen, later Flyway migrations are immutable. Check tenant
isolation, foreign keys, uniqueness, indexes, append-only evidence
semantics, safe defaults, and PostgreSQL 18 plus pgvector compatibility.

Files:

  • core/src/main/resources/db/migration/V26__assistant_message_turn_identity.sql
**/*.java

📄 CodeRabbit inference engine (AGENTS.md)

Apply IDE inspection only to edited backend Java files.

Files:

  • core/src/main/java/com/orgmemory/core/assistant/AssistantTurnRef.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantControllerStreamingTests.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantModelSelectionConcurrencyIntegrationTests.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantAnswerFeedbackConcurrencyIntegrationTests.java
  • core/src/main/java/com/orgmemory/core/assistant/AssistantConversationMessage.java
  • apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java
  • core/src/main/java/com/orgmemory/core/assistant/AssistantConversationService.java
  • core/src/test/java/com/orgmemory/core/assistant/AssistantConversationServiceTests.java
  • apps/api/src/test/java/com/orgmemory/api/assistant/AssistantTurnIdentityIntegrationTests.java
apps/api/src/main/java/**/*.java

⚙️ CodeRabbit configuration file

apps/api/src/main/java/**/*.java: Enforce the browser-BFF and resource-server boundaries. Authentication
must resolve an active internal actor through the explicit issuer and
subject binding. Reject identity, tenant, roles, or permissions supplied
by request payloads, JWT email, or untrusted JWT role claims.

Files:

  • apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java
🧠 Learnings (2)
📚 Learning: 2026-07-23T23:30:44.585Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 30
File: core/src/main/resources/db/migration/V32__evidence_scoped_graph_semantics.sql:0-0
Timestamp: 2026-07-23T23:30:44.585Z
Learning: For OrgMemory PostgreSQL Flyway migrations under core/src/main/resources/db/migration, do not recommend using `CREATE INDEX CONCURRENTLY` or `DROP INDEX CONCURRENTLY` inside application-owned Flyway migration SQL. Flyway’s schema-history connection may hold a transaction that can cause concurrent index operations to wait indefinitely (e.g., on a `virtualxid`), and docs/conventions.md forbids this pattern. If you need large production-table index replacement, pre-stage online index operations via the deployment pipeline (outside Flyway) rather than inside the migration; “ordinary” index replacement is acceptable for unreleased projections before production traffic.

Applied to files:

  • core/src/main/resources/db/migration/V26__assistant_message_turn_identity.sql
📚 Learning: 2026-07-26T05:46:47.443Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 61
File: apps/mcp/src/main/java/com/orgmemory/mcp/McpSecurityConfiguration.java:50-52
Timestamp: 2026-07-26T05:46:47.443Z
Learning: In OrgMemory, treat the `apps/mcp` and `apps/api` as independent protocol adapter modules. When adjusting OAuth/wire-level scopes, do not introduce a shared Java constant or create a code dependency from `apps/mcp` to `apps/api` solely to deduplicate scope values. Instead, keep OAuth/scope constants adapter-local (e.g., in the relevant adapter/security configuration classes) and ensure cross-adapter consistency via automated realm/OAuth/authorization tests, rather than via shared wiring-level constants or cross-module references.

Applied to files:

  • apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java
🪛 Squawk (2.61.0)
core/src/main/resources/db/migration/V26__assistant_message_turn_identity.sql

[warning] 18-20: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.

(require-concurrent-index-creation)

🔇 Additional comments (10)
core/src/main/java/com/orgmemory/core/assistant/AssistantTurnRef.java (1)

1-21: LGTM!

core/src/main/java/com/orgmemory/core/assistant/AssistantConversationMessage.java (1)

20-22: LGTM!

Also applies to: 49-67

core/src/main/resources/db/migration/V26__assistant_message_turn_identity.sql (1)

1-20: LGTM!

core/src/main/java/com/orgmemory/core/assistant/AssistantConversationService.java (1)

40-76: LGTM!

Also applies to: 95-135

core/src/test/java/com/orgmemory/core/assistant/AssistantConversationServiceTests.java (1)

4-10: LGTM!

Also applies to: 63-126, 191-191, 219-219, 240-240, 297-297, 370-383

apps/api/src/test/java/com/orgmemory/api/assistant/AssistantTurnIdentityIntegrationTests.java (1)

1-209: LGTM!

apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java (1)

13-13: LGTM!

Also applies to: 115-141, 423-435

apps/api/src/test/java/com/orgmemory/api/assistant/AssistantAnswerFeedbackConcurrencyIntegrationTests.java (1)

7-7: LGTM!

Also applies to: 120-122

apps/api/src/test/java/com/orgmemory/api/assistant/AssistantControllerStreamingTests.java (1)

26-26: LGTM!

Also applies to: 230-232, 267-267, 297-299

apps/api/src/test/java/com/orgmemory/api/assistant/AssistantModelSelectionConcurrencyIntegrationTests.java (1)

184-185: LGTM!


📝 Walkthrough

Walkthrough

The change introduces AssistantTurnRef, persists nullable turn identity on assistant messages, enforces per-role uniqueness for identified turns, and propagates the reference through conversation completion, streaming, concurrency, and integration tests.

Changes

Assistant turn identity

Layer / File(s) Summary
Turn identity contract and storage
core/src/main/java/com/orgmemory/core/assistant/AssistantTurnRef.java, core/src/main/java/com/orgmemory/core/assistant/AssistantConversationMessage.java, core/src/main/resources/db/migration/V26__assistant_message_turn_identity.sql
AssistantTurnRef pairs conversation and turn UUIDs. Messages store nullable turnId. The migration adds turn_id and a partial unique index for identified user and assistant messages.
Conversation turn lifecycle
core/src/main/java/com/orgmemory/core/assistant/AssistantConversationService.java, core/src/test/java/com/orgmemory/core/assistant/AssistantConversationServiceTests.java, apps/api/src/test/java/com/orgmemory/api/assistant/AssistantTurnIdentityIntegrationTests.java
Turn creation returns AssistantTurnRef. User and assistant messages persist the same turn ID. Tests cover pairing, concurrent turns, out-of-order completion, duplicate roles, and legacy messages.
Streaming integration and compatibility tests
apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java, apps/api/src/test/java/com/orgmemory/api/assistant/*
The streaming controller extracts the conversation ID from AssistantTurnRef and passes the reference to completion. Related API tests update mocked lifecycle calls and concurrency fixtures.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AssistantController
  participant AssistantConversationService
  participant AssistantConversationMessage
  participant PostgreSQL
  AssistantController->>AssistantConversationService: beginTurn(CurrentActor, conversationId, question)
  AssistantConversationService->>AssistantConversationMessage: persist user message with turnId
  AssistantConversationMessage->>PostgreSQL: insert conversation message
  AssistantConversationService-->>AssistantController: return AssistantTurnRef
  AssistantController->>AssistantConversationService: completeTurn(AssistantTurnRef)
  AssistantConversationService->>AssistantConversationMessage: persist assistant message with turnId
  AssistantConversationMessage->>PostgreSQL: insert paired message
Loading

Possibly related PRs

  • kl3inIT/OrgMemory#57: Both changes evolve assistant turn handling across the conversation service, message model, controller, and tests.
  • kl3inIT/OrgMemory#289: Both changes propagate server-generated assistant identity through persistence and streaming completion.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: adding explicit identity to assistant conversation turns.
Description check ✅ Passed The description covers the change, rationale, verification evidence, scope, and known limitation, but it omits the required Documentation impact section.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/assistant-turn-identity

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Tegami

This repository uses Tegami to manage releases. When your changes affect published packages, add a changelog file under .tegami/ before merging.

Create a changelog → · Changelog format

Release preview

Package Bump Version
orgmemory minor 0.2.00.3.0

This PR does not add changelog files. Pending changelogs from other branches are included in the preview above.

Run pnpm run tegami locally to create a changelog interactively.

Managed by Tegami.

@kl3inIT
kl3inIT merged commit bf91a7c into main Aug 6, 2026
18 checks passed
@kl3inIT
kl3inIT deleted the feat/assistant-turn-identity branch August 6, 2026 08:55
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.

1 participant