-
Notifications
You must be signed in to change notification settings - Fork 1
Keep a team-mode area open for the manager's next message #257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
LinseCed
wants to merge
1
commit into
dev
Choose a base branch
from
fix/team-mode-areas-across-turns
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
40 changes: 40 additions & 0 deletions
40
src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/OpenAreas.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| package com.sprintstart.sprintstartbackend.onboarding.service | ||
|
|
||
| /** | ||
| * The team areas whose tools are mounted while one manager message is answered. | ||
| * | ||
| * Two things put an area here. The manager's previous message may have opened it ([carriedOver]), which is | ||
| * what makes "yes, send it" work: drafting something opens an area, approving it comes a message later, | ||
| * and the transcript is text only, so nothing else remembers that the area was open. And the model can open | ||
| * one itself during this turn ([open]). | ||
| * | ||
| * Only what this turn opened is remembered for the next one ([openedThisTurn]). What was carried over is not | ||
| * carried again, so an area stays open for one further message and a long conversation does not slowly | ||
| * mount every area's tools — the reason areas exist. | ||
| */ | ||
| internal class OpenAreas( | ||
| carriedOver: Set<TeamArea> = emptySet(), | ||
| ) { | ||
| /** Every area mounted so far, this turn's and the last one's. Read again on each hop. */ | ||
| val mounted: MutableSet<TeamArea> = carriedOver.toMutableSet() | ||
|
|
||
| /** The areas the model opened during this turn. */ | ||
| val openedThisTurn: MutableSet<TeamArea> = mutableSetOf() | ||
|
|
||
| fun open(area: TeamArea) { | ||
| mounted.add(area) | ||
| openedThisTurn.add(area) | ||
| } | ||
| } | ||
|
|
||
| /** The stored form of the areas a reply opened: their names, comma-separated and sorted; null when none. */ | ||
| internal fun Set<TeamArea>.encoded(): String? = | ||
| takeIf { it.isNotEmpty() }?.sortedBy { it.name }?.joinToString(",") { it.name } | ||
|
|
||
| /** The areas a stored [encoded] value names. A name that is no longer an area is dropped, never an error. */ | ||
| internal fun String?.toTeamAreas(): Set<TeamArea> = | ||
| this | ||
| ?.split(',') | ||
| ?.mapNotNull { name -> TeamArea.entries.firstOrNull { it.name == name.trim() } } | ||
| ?.toSet() | ||
| .orEmpty() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5 changes: 5 additions & 0 deletions
5
src/main/resources/db/migration/V20__add_buddy_team_message_opened_areas.sql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| -- The team areas a buddy reply opened, so the manager's next message still has the tools that area mounts. | ||
| -- Nullable on purpose: existing messages opened nothing that matters, and Hibernate (ddl-auto: update) can add | ||
| -- a nullable column to a populated table, which it cannot do for NOT NULL ones. Idempotent. | ||
| ALTER TABLE buddy_team_messages | ||
| ADD COLUMN IF NOT EXISTS opened_areas VARCHAR(255); |
56 changes: 56 additions & 0 deletions
56
...om/sprintstart/sprintstartbackend/onboarding/repository/BuddyTeamMessageRepositoryTest.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package com.sprintstart.sprintstartbackend.onboarding.repository | ||
|
|
||
| import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyMessageRole | ||
| import com.sprintstart.sprintstartbackend.onboarding.model.entity.BuddyTeamMessage | ||
| import com.sprintstart.sprintstartbackend.onboarding.model.entity.BuddyTeamSession | ||
| import com.sprintstart.sprintstartbackend.shared.crypto.CryptoConfiguration | ||
| import jakarta.persistence.EntityManager | ||
| import org.assertj.core.api.Assertions.assertThat | ||
| import org.junit.jupiter.api.Test | ||
| import org.springframework.beans.factory.annotation.Autowired | ||
| import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest | ||
| import org.springframework.context.annotation.Import | ||
| import org.springframework.test.context.ActiveProfiles | ||
| import java.time.Instant | ||
| import java.util.UUID | ||
|
|
||
| /** | ||
| * What a team reply opened has to survive the trip to the database and back: the next message reads it to | ||
| * decide which tools to mount, and a value that came back changed or empty would silently bring the | ||
| * "yes, send it" failure back. | ||
| */ | ||
| @ActiveProfiles("test") | ||
| @DataJpaTest | ||
| @Import(CryptoConfiguration::class) | ||
| class BuddyTeamMessageRepositoryTest { | ||
| @Autowired | ||
| private lateinit var repository: BuddyTeamMessageRepository | ||
|
|
||
| @Autowired | ||
| private lateinit var entityManager: EntityManager | ||
|
|
||
| private fun session(): BuddyTeamSession = | ||
| BuddyTeamSession(userId = UUID.randomUUID(), projectId = UUID.randomUUID()).also { entityManager.persist(it) } | ||
|
|
||
| private fun message(session: BuddyTeamSession, offsetSeconds: Long, opened: String?) = | ||
| BuddyTeamMessage( | ||
| session = session, | ||
| role = BuddyMessageRole.ASSISTANT, | ||
| content = "reply $offsetSeconds", | ||
| createdAt = Instant.parse("2026-09-21T10:00:00Z").plusSeconds(offsetSeconds), | ||
| openedAreas = opened, | ||
| ) | ||
|
|
||
| @Test | ||
| fun `the areas a reply opened come back exactly as stored, and none comes back as null`() { | ||
| val session = session() | ||
| repository.save(message(session, 1, "ARRIVAL,KNOWLEDGE")) | ||
| repository.save(message(session, 2, null)) | ||
| entityManager.flush() | ||
| entityManager.clear() | ||
|
|
||
| val stored = repository.findAllBySessionIdOrderByCreatedAtAsc(session.id) | ||
|
|
||
| assertThat(stored.map { it.openedAreas }).containsExactly("ARRIVAL,KNOWLEDGE", null) | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still makes the fix conditional on the model choosing to call
open_area.areas.openedThisTurnis populated only by anopen_areatool call. The live reproduction shows that the model can instead return a draft or nonsensical final answer without openingKNOWLEDGE. This line then storesnull, so the manager’s next message - such as “send that draft” - again has no escalation action tool mounted. The original bug therefore remains possible and has already reproduced against this head.The new
open_areadescription is helpful guidance, but prompt text is not a reliable state transition or enforcement mechanism.Please make area continuation deterministic on the backend. For example, require a structured area-selection/tool step before accepting an area-dependent final response, or otherwise prevent an actionable workflow from finalizing until the relevant area has been opened or an action proposal has actually been emitted. A response must also never be allowed to claim that an action was completed unless the backend created/executed the corresponding proposal.