diff --git a/basilisk/conversation/conversation_model.py b/basilisk/conversation/conversation_model.py index c38d4067d..99ba971c3 100644 --- a/basilisk/conversation/conversation_model.py +++ b/basilisk/conversation/conversation_model.py @@ -218,9 +218,12 @@ class Conversation(BaseModel): version: int = Field(default=BSKC_VERSION, ge=0, le=BSKC_VERSION) group_participants: list[GroupParticipant] = Field(default_factory=list) # Private: populated by GroupConversationPresenter before each chain step. - # Maps str(profile_id) -> profile name for display in history. + # Maps str(profile_id) -> profile name for attribution in group context. # Excluded from BSKC serialization automatically by PrivateAttr. _profile_name_map: dict[str, str] = PrivateAttr(default_factory=dict) + # Private: set to str(profile_id) of the participant being called next so + # get_messages() can restructure the history from that participant's POV. + _current_group_participant_id: str | None = PrivateAttr(default=None) @model_validator(mode="before") @classmethod diff --git a/basilisk/presenters/group_conversation_presenter.py b/basilisk/presenters/group_conversation_presenter.py index 0390bf427..39b80bf4c 100644 --- a/basilisk/presenters/group_conversation_presenter.py +++ b/basilisk/presenters/group_conversation_presenter.py @@ -156,10 +156,14 @@ def _submit_next_participant(self): participant = self.participants[self._current_participant_index] position = self._current_participant_index - # Populate the profile name map so base_engine prefixes responses. + # Tell get_messages() which participant is being called so it can + # rebuild history from that participant's point of view. self.conversation._profile_name_map = { str(p.profile_id): p.name for p in self.participants } + self.conversation._current_group_participant_id = str( + participant.profile_id + ) # Resolve account and engine for this participant account = self._resolve_account(participant) @@ -178,14 +182,15 @@ def _submit_next_participant(self): if participant.system_prompt: system_message = SystemMessage(content=participant.system_prompt) - # For debate rounds > 0 use an empty sentinel request so the model - # responds only based on conversation history. - if self._current_round > 0: - request_content = "" - else: + # Only the first participant of the first round gets the original + # prompt. All others receive an empty sentinel so the model continues + # from the existing conversation history without a duplicate question. + if self._current_round == 0 and position == 0: request_content = ( self._pending_request.content if self._pending_request else "" ) + else: + request_content = "" request_attachments = ( self._pending_request.attachments if ( @@ -336,6 +341,7 @@ def _on_completion_error(self, error_message: str): def _on_chain_complete(self): """Called when the full chain has completed or been stopped.""" self.conversation._profile_name_map = {} + self.conversation._current_group_participant_id = None self.view.set_active_participant(None) self.view.on_chain_complete() diff --git a/basilisk/provider_engine/base_engine.py b/basilisk/provider_engine/base_engine.py index 7c9d538b2..34b584665 100644 --- a/basilisk/provider_engine/base_engine.py +++ b/basilisk/provider_engine/base_engine.py @@ -115,55 +115,80 @@ def get_messages( ) -> list[Message]: """Prepares message history for API requests. - For group chat blocks (``group_position > 0``) the user request is - **not** re-emitted because the same prompt was already sent by the - block at position 0. When ``_profile_name_map`` is set on the - conversation, assistant responses are prefixed with the participant - name so each model knows who said what. + For group chat conversations, the history is restructured from each + participant's point of view: + - The current participant's own previous responses appear as ASSISTANT + turns (natural continuation). + - Other participants' responses appear as USER turns, attributed with + ``[name]: content`` so the model sees them as external input rather + than its own prior output. This prevents models from imitating the + ``[name]:`` prefix format in their own responses. Args: new_block: Current message block being processed. conversation: Full conversation history. system_message: Optional system-level instruction message. - stop_block_index: Optional index to stop processing messages at. If None, all messages are processed. + stop_block_index: Stop processing history at this index (exclusive). Returns: List of prepared messages in provider-specific format. """ + from basilisk.conversation.conversation_model import ( + Message, + MessageRoleEnum, + ) + messages = [] if system_message: messages.append(self.prepare_message_request(system_message)) name_map: dict[str, str] = getattr( conversation, "_profile_name_map", {} ) + current_pid: str | None = getattr( + conversation, "_current_group_participant_id", None + ) for i, block in enumerate(conversation.messages): if stop_block_index is not None and i >= stop_block_index: break if not block.response: continue - # For group blocks after the first participant, skip re-emitting - # the user request (position 0 already emitted it). - skip_user_request = ( - block.group_position is not None and block.group_position > 0 - ) - if not skip_user_request: - messages.append(self.prepare_message_request(block.request)) - # Optionally prefix the response with the participant name. - response = block.response - if name_map and block.profile_id is not None: - profile_name = name_map.get(str(block.profile_id)) - if profile_name: - from basilisk.conversation.conversation_model import ( - Message, - MessageRoleEnum, + if current_pid is not None and block.group_id is not None: + # Group block: rebuild history from this participant's POV. + pid_str = str(block.profile_id) if block.profile_id else None + if pid_str == current_pid: + # Own previous response: include request (if non-empty) + # then own response as ASSISTANT. + if block.request.content: + messages.append( + self.prepare_message_request(block.request) + ) + messages.append( + self.prepare_message_response(block.response) ) - - response = Message( - role=MessageRoleEnum.ASSISTANT, - content=f"[{profile_name}]: {response.content}", + else: + # Another participant's response: frame as USER turn so + # the model doesn't imitate the attribution format. + other_name = ( + name_map.get(pid_str, "Other") if pid_str else "Other" + ) + attributed = Message( + role=MessageRoleEnum.USER, + content=f"[{other_name}]: {block.response.content}", ) - messages.append(self.prepare_message_response(response)) - messages.append(self.prepare_message_request(new_block.request)) + messages.append(self.prepare_message_request(attributed)) + else: + # Standard (non-group) block. + skip_user = ( + block.group_position is not None + and block.group_position > 0 + ) + if not skip_user: + messages.append(self.prepare_message_request(block.request)) + messages.append(self.prepare_message_response(block.response)) + # Append the new block's request only when it carries content. + # For group follow-ups the last USER turn is already an attribution. + if new_block.request.content: + messages.append(self.prepare_message_request(new_block.request)) return messages @abstractmethod diff --git a/basilisk/views/history_msg_text_ctrl.py b/basilisk/views/history_msg_text_ctrl.py index 9d1b6a580..697af9036 100644 --- a/basilisk/views/history_msg_text_ctrl.py +++ b/basilisk/views/history_msg_text_ctrl.py @@ -231,9 +231,11 @@ def display_new_block( """ absolute_length = self.GetLastPosition() new_block_ref = weakref.ref(new_block) - is_group_followup = ( - new_block.group_position is not None - and new_block.group_position > 0 + # Skip user request for group follow-up blocks: + # - any block with group_position > 0 (not the chain opener), or + # - position-0 blocks in subsequent rounds (empty sentinel request). + is_group_followup = new_block.group_id is not None and ( + new_block.group_position > 0 or not new_block.request.content ) if not self.IsEmpty(): absolute_length = self.append_suffix(new_block_ref, absolute_length) diff --git a/tests/presenters/test_group_conversation_presenter.py b/tests/presenters/test_group_conversation_presenter.py index a99ec3278..d2ab04cd8 100644 --- a/tests/presenters/test_group_conversation_presenter.py +++ b/tests/presenters/test_group_conversation_presenter.py @@ -206,6 +206,32 @@ def test_debate_progresses_through_rounds(self, mocker): # Chain done assert not presenter._is_running_chain + def test_second_participant_in_round_0_uses_empty_request(self, mocker): + """Position > 0 in round 0 must use empty content (no duplicate question).""" + presenter = _make_presenter() + + mock_account = MagicMock() + mock_account.provider = MagicMock() + mock_account.provider.id = "openai" + mock_account.provider.engine_cls = MagicMock(return_value=MagicMock()) + + mocker.patch.object( + presenter, "_resolve_account", return_value=mock_account + ) + mock_start = mocker.patch.object( + presenter.completion_handler, "start_completion" + ) + + presenter.on_submit() + + # Complete round 0, participant 0 + presenter._on_completion_end(True) + + # Round 0, participant 1 — request must be empty, not the original prompt + assert mock_start.call_count == 2 + call_kwargs = mock_start.call_args.kwargs + assert call_kwargs["new_block"].request.content == "" + def test_debate_uses_empty_sentinel_in_round_1(self, mocker): """Subsequent debate rounds should use an empty request content.""" presenter = _make_presenter() diff --git a/tests/test_group_message_block.py b/tests/test_group_message_block.py index 7d25b7a47..201962811 100644 --- a/tests/test_group_message_block.py +++ b/tests/test_group_message_block.py @@ -161,6 +161,18 @@ def test_profile_name_map_not_in_serialization(self): data = json.loads(conv.model_dump_json()) assert "_profile_name_map" not in data + def test_current_group_participant_id_defaults_none(self): + """_current_group_participant_id defaults to None (private attr).""" + conv = Conversation() + assert conv._current_group_participant_id is None + + def test_current_group_participant_id_not_in_serialization(self): + """_current_group_participant_id is excluded from JSON serialization.""" + conv = Conversation() + conv._current_group_participant_id = "some-uuid" + data = json.loads(conv.model_dump_json()) + assert "_current_group_participant_id" not in data + def test_group_participants_excluded_from_bskc_when_empty(self): """group_participants absent from JSON payload when empty is fine.""" conv = Conversation()