feat: conv visibility

This commit is contained in:
Alex committed 2026-06-08 23:46:28 +01:00
1 parent 6d4d96d12d
commit 82bacfdb2b
18 files changed
+361 -110

No files matched your search

@@ -0,0 +1,59 @@
"""0016 conversation visibility — separate "persisted" from "shown in sidebar".
Adds ``conversations.visibility`` (``listed`` | ``hidden``) so a conversation
can be stored without surfacing in the owner's sidebar. Until now display was
inferred from identity columns: ``(api_key IS NULL OR agent_id IS NOT NULL)``.
The backfill reproduces that exact predicate so existing sidebars are
unchanged; new rows set the value explicitly at write time.
Revision ID: 0016_conversation_visibility
Revises: 0015_token_usage_model_id
"""
from typing import Sequence, Union
from alembic import op
revision: str = "0016_conversation_visibility"
down_revision: Union[str, None] = "0015_token_usage_model_id"
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None
def upgrade() -> None:
op.execute(
"ALTER TABLE conversations "
"ADD COLUMN visibility TEXT NOT NULL DEFAULT 'listed';"
)
# NOT VALID first so the ACCESS EXCLUSIVE lock doesn't scan the table;
# VALIDATE then runs under a weaker lock that allows concurrent reads/writes.
op.execute(
"ALTER TABLE conversations "
"ADD CONSTRAINT conversations_visibility_chk "
"CHECK (visibility IN ('listed', 'hidden')) NOT VALID;"
)
op.execute(
"ALTER TABLE conversations VALIDATE CONSTRAINT conversations_visibility_chk;"
)
# Preserve current sidebars: the old display heuristic showed a
# conversation when (api_key IS NULL OR agent_id IS NOT NULL); hide the rest.
op.execute(
"UPDATE conversations SET visibility = 'hidden' "
"WHERE NOT (api_key IS NULL OR agent_id IS NOT NULL);"
)
# Matches the sidebar query: user_id + visibility, newest first.
op.execute(
"CREATE INDEX conversations_user_listed_idx "
'ON conversations (user_id, date DESC) '
"WHERE visibility = 'listed';"
)
def downgrade() -> None:
op.execute("DROP INDEX IF EXISTS conversations_user_listed_idx;")
op.execute(
"ALTER TABLE conversations "
"DROP CONSTRAINT IF EXISTS conversations_visibility_chk;"
)
op.execute("ALTER TABLE conversations DROP COLUMN IF EXISTS visibility;")
+9 -1
View File
@@ -8,6 +8,7 @@ from application.api import api
from application.api.answer.routes.base import answer_ns, BaseAnswerResource
from application.api.answer.services.persistence_policy import resolve_persistence
from application.api.answer.services.stream_processor import StreamProcessor
logger = logging.getLogger(__name__)
@@ -117,6 +118,12 @@ class AnswerResource(Resource, BaseAnswerResource):
if error := self.check_usage(processor.agent_config):
return error
should_persist, visibility = resolve_persistence(
display_flag=data.get("save_conversation"),
api_key=data.get("api_key"),
is_shared_usage=processor.is_shared_usage,
persist_flag=data.get("persist"),
)
stream = self.complete_stream(
question=data["question"],
agent=agent,
@@ -125,7 +132,8 @@ class AnswerResource(Resource, BaseAnswerResource):
decoded_token=processor.decoded_token,
isNoneDoc=data.get("isNoneDoc"),
index=None,
should_save_conversation=data.get("save_conversation", True),
should_persist=should_persist,
visibility=visibility,
agent_id=processor.agent_id,
is_shared_usage=processor.is_shared_usage,
shared_token=processor.shared_token,
+12 -6
View File
@@ -183,7 +183,8 @@ class BaseAnswerResource:
decoded_token: Dict[str, Any],
isNoneDoc: bool = False,
index: Optional[int] = None,
should_save_conversation: bool = True,
should_persist: bool = True,
visibility: str = "listed",
attachment_ids: Optional[List[str]] = None,
agent_id: Optional[str] = None,
is_shared_usage: bool = False,
@@ -204,7 +205,8 @@ class BaseAnswerResource:
decoded_token: Decoded JWT token
isNoneDoc: Flag for document-less responses
index: Index of message to update
should_save_conversation: Whether to persist the conversation
should_persist: Whether to persist the conversation
visibility: ``listed`` (sidebar) or ``hidden`` for a new conversation
attachment_ids: List of attachment IDs
agent_id: ID of agent used
is_shared_usage: Flag for shared agent usage
@@ -232,7 +234,7 @@ class BaseAnswerResource:
# mid-stream still leaves the question queryable. Continuations
# reuse the original placeholder.
reserved_message_id: Optional[str] = None
wal_eligible = should_save_conversation and not _continuation
wal_eligible = should_persist and not _continuation
if wal_eligible:
try:
reservation = self.conversation_service.save_user_question(
@@ -244,6 +246,7 @@ class BaseAnswerResource:
agent_id=agent_id,
is_shared_usage=is_shared_usage,
shared_token=shared_token,
visibility=visibility,
model_id=model_id or self.default_model_id,
request_id=request_id,
index=index,
@@ -489,7 +492,7 @@ class BaseAnswerResource:
continuation = getattr(agent, "_pending_continuation", None)
if continuation:
# First-turn pause needs a conversation row to attach to.
if not conversation_id and should_save_conversation:
if not conversation_id and should_persist:
try:
provider = (
get_provider_from_model_id(
@@ -531,6 +534,7 @@ class BaseAnswerResource:
agent_id=agent_id,
is_shared_usage=is_shared_usage,
shared_token=shared_token,
visibility=visibility,
)
)
except Exception as e:
@@ -678,7 +682,7 @@ class BaseAnswerResource:
# Title-gen only; agent stream tokens live on ``agent.llm``.
llm._token_usage_source = "title"
if should_save_conversation:
if should_persist:
if reserved_message_id is not None:
self.conversation_service.finalize_message(
reserved_message_id,
@@ -717,6 +721,7 @@ class BaseAnswerResource:
shared_token=shared_token,
attachment_ids=attachment_ids,
metadata=query_metadata if query_metadata else None,
visibility=visibility,
)
# Persist compression metadata/summary if it exists and wasn't saved mid-execution
compression_meta = getattr(agent, "compression_metadata", None)
@@ -823,7 +828,7 @@ class BaseAnswerResource:
# else journal ``error`` so a reconnecting client sees a
# failed terminal state instead of a blank "success".
finalized_complete = False
if should_save_conversation and response_full:
if should_persist and response_full:
try:
if isNoneDoc:
for doc in source_log_docs:
@@ -903,6 +908,7 @@ class BaseAnswerResource:
shared_token=shared_token,
attachment_ids=attachment_ids,
metadata=query_metadata if query_metadata else None,
visibility=visibility,
)
# No journal row to gate, but flag the save as
# successful for symmetry with the WAL path.
+9 -1
View File
@@ -8,6 +8,7 @@ from application.api import api
from application.api.answer.routes.base import answer_ns, BaseAnswerResource
from application.api.answer.services.persistence_policy import resolve_persistence
from application.api.answer.services.stream_processor import StreamProcessor
logger = logging.getLogger(__name__)
@@ -135,6 +136,12 @@ class StreamResource(Resource, BaseAnswerResource):
if error := self.check_usage(processor.agent_config):
return error
should_persist, visibility = resolve_persistence(
display_flag=data.get("save_conversation"),
api_key=data.get("api_key"),
is_shared_usage=processor.is_shared_usage,
persist_flag=data.get("persist"),
)
return Response(
self.complete_stream(
question=data["question"],
@@ -144,7 +151,8 @@ class StreamResource(Resource, BaseAnswerResource):
decoded_token=processor.decoded_token,
isNoneDoc=data.get("isNoneDoc"),
index=data.get("index"),
should_save_conversation=data.get("save_conversation", True),
should_persist=should_persist,
visibility=visibility,
attachment_ids=data.get("attachments", []),
agent_id=processor.agent_id,
is_shared_usage=processor.is_shared_usage,
@@ -78,6 +78,7 @@ class ConversationService:
shared_token: Optional[str] = None,
attachment_ids: Optional[List[str]] = None,
metadata: Optional[Dict[str, Any]] = None,
visibility: str = "listed",
) -> str:
"""Save or update a conversation in Postgres.
@@ -182,6 +183,7 @@ class ConversationService:
if (resolved_agent_id and is_shared_usage)
else None
),
visibility=visibility,
)
conv_pg_id = str(conv["id"])
append_payload = dict(message_payload)
@@ -202,6 +204,7 @@ class ConversationService:
shared_token: Optional[str] = None,
model_id: Optional[str] = None,
request_id: Optional[str] = None,
visibility: str = "listed",
status: str = "pending",
index: Optional[int] = None,
) -> Dict[str, str]:
@@ -257,6 +260,7 @@ class ConversationService:
if (resolved_agent_id and is_shared_usage)
else None
),
visibility=visibility,
)
conv_pg_id = str(conv["id"])
@@ -0,0 +1,45 @@
"""Resolve whether an answer is persisted and whether it lists in the sidebar.
Persistence (is a row written at all?) and visibility (does it show in the
owner's sidebar?) used to be one decision driven by the request-level
``save_conversation`` flag plus an identity heuristic. They are now separate:
conversations persist by default everywhere, while ``save_conversation`` only
controls sidebar visibility.
"""
from typing import Optional, Tuple
VISIBILITY_LISTED = "listed"
VISIBILITY_HIDDEN = "hidden"
def resolve_persistence(
*,
display_flag: Optional[bool],
api_key: Optional[str] = None,
is_shared_usage: bool = False,
persist_flag: Optional[bool] = None,
) -> Tuple[bool, str]:
"""Resolve ``(should_persist, visibility)`` for an answer request.
Args:
display_flag: Request-level ``save_conversation`` (``None`` if the
caller omitted it). Forces visibility on/off when provided.
api_key: Present for programmatic/agent callers (including the
OpenAI-compatible ``/v1`` endpoint), absent for first-party
session users.
is_shared_usage: True for shared-agent/widget traffic.
persist_flag: Explicit persistence opt-out (``False`` to skip writing
a row, e.g. stateless tool rounds that would orphan one). ``None``
keeps the always-persist default.
Returns:
``(should_persist, visibility)`` where ``visibility`` is
``"listed"`` or ``"hidden"``.
"""
should_persist = True if persist_flag is None else bool(persist_flag)
# Only genuine first-party interactive turns default to the sidebar.
default_display = api_key is None and not is_shared_usage
display = default_display if display_flag is None else bool(display_flag)
visibility = VISIBILITY_LISTED if display else VISIBILITY_HIDDEN
return should_persist, visibility
+1 -1
View File
@@ -76,7 +76,7 @@ class DeleteAllConversations(Resource):
@conversations_ns.route("/get_conversations")
class GetConversations(Resource):
@api.doc(
description="Retrieve a list of the latest 30 conversations (excluding API key conversations)",
description="Retrieve a list of the latest 30 sidebar conversations (visibility = listed)",
)
def get(self):
decoded_token = request.decoded_token
+21 -7
View File
@@ -15,6 +15,7 @@ from typing import Any, Dict, Generator, Optional
from flask import Blueprint, jsonify, make_response, request, Response
from application.api.answer.routes.base import BaseAnswerResource
from application.api.answer.services.persistence_policy import resolve_persistence
from application.api.answer.services.stream_processor import StreamProcessor
from application.api.v1.translator import (
translate_request,
@@ -157,7 +158,14 @@ def chat_completions():
if usage_error:
return usage_error
should_save_conversation = bool(internal_data.get("save_conversation", False))
# v1 always persists (unless the translator opted out for a stateless
# tool round); the agent owner's sidebar only lists it on explicit
# ``docsgpt.save_conversation: true``.
should_persist, visibility = resolve_persistence(
display_flag=internal_data.get("save_conversation"),
api_key=internal_data.get("api_key"),
persist_flag=internal_data.get("persist"),
)
# Only strip leaked reasoning from content for structured requests -- the
# only path where models echo reasoning into content -- so legitimate
# answers that mention the marker text are never corrupted.
@@ -174,7 +182,8 @@ def chat_completions():
processor,
model_name,
continuation,
should_save_conversation,
should_persist,
visibility,
strip_reasoning_leak,
),
mimetype="text/event-stream",
@@ -191,7 +200,8 @@ def chat_completions():
processor,
model_name,
continuation,
should_save_conversation,
should_persist,
visibility,
strip_reasoning_leak,
)
@@ -222,7 +232,8 @@ def _stream_response(
processor: StreamProcessor,
model_name: str,
continuation: Optional[Dict],
should_save_conversation: bool,
should_persist: bool,
visibility: str,
strip_reasoning_leak: bool = False,
) -> Generator[str, None, None]:
"""Generate translated SSE chunks for streaming response."""
@@ -237,7 +248,8 @@ def _stream_response(
agent_id=processor.agent_id,
model_id=processor.model_id,
model_user_id=processor.model_user_id,
should_save_conversation=should_save_conversation,
should_persist=should_persist,
visibility=visibility,
_continuation=continuation,
)
@@ -285,7 +297,8 @@ def _non_stream_response(
processor: StreamProcessor,
model_name: str,
continuation: Optional[Dict],
should_save_conversation: bool,
should_persist: bool,
visibility: str,
strip_reasoning_leak: bool = False,
) -> Response:
"""Collect full response and return as single JSON."""
@@ -298,7 +311,8 @@ def _non_stream_response(
agent_id=processor.agent_id,
model_id=processor.model_id,
model_user_id=processor.model_user_id,
should_save_conversation=should_save_conversation,
should_persist=should_persist,
visibility=visibility,
_continuation=continuation,
)
+10 -8
View File
@@ -263,13 +263,15 @@ def translate_request(
# rebuilt from the resent messages instead of server-side state.
"messages": messages,
}
# Stateful continuations (with a conversation_id) default to True so the
# final turn / WAL row is persisted. Stateless continuations (no
# conversation_id, e.g. opencode) default to False — otherwise every
# tool round persists an orphan conversation with an empty question. An
# explicit ``docsgpt.save_conversation`` override is honoured either way.
# Persistence: stateful continuations (carrying a conversation_id)
# persist the final turn; stateless ones (no conversation_id, e.g.
# opencode) skip it, else every tool round writes an orphan conversation
# with an empty question. ``docsgpt.persist`` overrides. Visibility is
# separate — hidden unless ``docsgpt.save_conversation`` opts in.
docsgpt_ext = data.get("docsgpt", {})
result["persist"] = bool(docsgpt_ext.get("persist", bool(conversation_id)))
result["save_conversation"] = bool(
data.get("docsgpt", {}).get("save_conversation", bool(conversation_id))
docsgpt_ext.get("save_conversation", False)
)
# Carry tools forward for next iteration
if data.get("tools"):
@@ -303,8 +305,8 @@ def translate_request(
"question": question,
"api_key": api_key,
"history": json.dumps(history),
# Conversations are NOT persisted by default on the v1 endpoint.
# Callers opt in via ``docsgpt.save_conversation: true``.
# v1 conversations always persist; ``save_conversation`` only controls
# whether they list in the agent owner's sidebar (default hidden).
"save_conversation": bool(docsgpt.get("save_conversation", False)),
}
+3
View File
@@ -326,6 +326,9 @@ conversations_table = Table(
Column("is_shared_usage", Boolean, nullable=False, server_default="false"),
Column("shared_token", Text),
Column("shared_with", ARRAY(Text), nullable=False, server_default="{}"),
# "listed" shows in the owner's sidebar; "hidden" persists silently
# (agent/API/OpenAI-compat traffic). See migration 0016.
Column("visibility", Text, nullable=False, server_default="listed"),
Column("compression_metadata", JSONB),
Column("date", DateTime(timezone=True), nullable=False, server_default=func.now()),
Column("created_at", DateTime(timezone=True), nullable=False, server_default=func.now()),
@@ -45,7 +45,7 @@ class MessageUpdateOutcome(str, Enum):
def _message_row_to_dict(row) -> dict:
"""Like ``row_to_dict`` but renames the DB column ``message_metadata``
back to the public API key ``metadata`` so callers keep the Mongo-era
shape. See migration 0016 for the column rename rationale."""
shape. See migration 0001_initial for the column rename rationale."""
out = row_to_dict(row)
if "message_metadata" in out:
out["metadata"] = out.pop("message_metadata")
@@ -149,6 +149,7 @@ class ConversationsRepository:
api_key: str | None = None,
is_shared_usage: bool = False,
shared_token: str | None = None,
visibility: str = "listed",
legacy_mongo_id: str | None = None,
) -> dict:
"""Create a new conversation.
@@ -162,6 +163,7 @@ class ConversationsRepository:
values: dict = {
"user_id": user_id,
"name": name,
"visibility": visibility,
}
# ``agent_id`` may arrive as a Mongo ObjectId during the dual-write
# window; resolve to a UUID (or drop silently if not yet backfilled).
@@ -247,15 +249,16 @@ class ConversationsRepository:
return row_to_dict(row) if row is not None else None
def list_for_user(self, user_id: str, limit: int = 30) -> list[dict]:
"""List conversations for a user, most recent first.
"""List a user's sidebar conversations, most recent first.
Mirrors the Mongo query: either no api_key or agent_id exists.
Only ``visibility = 'listed'`` rows surface; agent/API/OpenAI-compat
traffic persists as ``'hidden'`` and is excluded.
"""
result = self._conn.execute(
text(
"SELECT * FROM conversations "
"WHERE user_id = :user_id "
"AND (api_key IS NULL OR agent_id IS NOT NULL) "
"AND visibility = 'listed' "
"ORDER BY date DESC LIMIT :limit"
),
{"user_id": user_id, "limit": limit},
@@ -302,7 +305,7 @@ class ConversationsRepository:
" LIMIT 1 "
") mt ON TRUE "
"WHERE c.user_id = :user_id "
"AND (c.api_key IS NULL OR c.agent_id IS NOT NULL) "
"AND c.visibility = 'listed' "
"ORDER BY c.date DESC LIMIT :limit"
),
{
+6 -6
View File
@@ -139,7 +139,7 @@ class TestCompleteStreamMethod:
conversation_id=None,
user_api_key=None,
decoded_token=decoded_token,
should_save_conversation=False,
should_persist=False,
)
)
@@ -171,7 +171,7 @@ class TestCompleteStreamMethod:
conversation_id=None,
user_api_key=None,
decoded_token=decoded_token,
should_save_conversation=False,
should_persist=False,
)
)
@@ -197,7 +197,7 @@ class TestCompleteStreamMethod:
conversation_id=None,
user_api_key=None,
decoded_token=decoded_token,
should_save_conversation=False,
should_persist=False,
)
)
@@ -239,7 +239,7 @@ class TestCompleteStreamMethod:
conversation_id=None,
user_api_key=None,
decoded_token=decoded_token,
should_save_conversation=True,
should_persist=True,
)
)
@@ -299,7 +299,7 @@ class TestCompleteStreamMethod:
conversation_id=None, # caller had no conv yet
user_api_key=None,
decoded_token={"sub": "user-prop"},
should_save_conversation=True,
should_persist=True,
)
)
@@ -330,7 +330,7 @@ class TestCompleteStreamMethod:
conversation_id="conv-1",
user_api_key=None,
decoded_token={"sub": "user123"},
should_save_conversation=True,
should_persist=True,
)
)
@@ -0,0 +1,44 @@
"""Unit tests for ``resolve_persistence`` (persist + sidebar visibility)."""
import pytest
from application.api.answer.services.persistence_policy import (
VISIBILITY_HIDDEN,
VISIBILITY_LISTED,
resolve_persistence,
)
@pytest.mark.unit
class TestResolvePersistence:
def test_first_party_default_lists(self):
persist, visibility = resolve_persistence(display_flag=None)
assert persist is True
assert visibility == VISIBILITY_LISTED
def test_api_key_caller_defaults_hidden(self):
persist, visibility = resolve_persistence(display_flag=None, api_key="k-1")
assert persist is True
assert visibility == VISIBILITY_HIDDEN
def test_shared_usage_defaults_hidden(self):
_, visibility = resolve_persistence(display_flag=None, is_shared_usage=True)
assert visibility == VISIBILITY_HIDDEN
def test_explicit_display_true_lists_api_key_caller(self):
_, visibility = resolve_persistence(display_flag=True, api_key="k-1")
assert visibility == VISIBILITY_LISTED
def test_explicit_display_false_hides_first_party(self):
persist, visibility = resolve_persistence(display_flag=False)
# Still persisted, just not shown in the sidebar.
assert persist is True
assert visibility == VISIBILITY_HIDDEN
def test_persist_opt_out(self):
persist, _ = resolve_persistence(display_flag=None, persist_flag=False)
assert persist is False
def test_persist_defaults_true_for_hidden_api_caller(self):
persist, _ = resolve_persistence(display_flag=False, api_key="k-1")
assert persist is True
+22 -22
View File
@@ -98,7 +98,7 @@ class TestCompleteStreamToolCalls:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=False,
should_persist=False,
)
)
tool_chunks = [s for s in stream if '"type": "tool_calls"' in s]
@@ -124,7 +124,7 @@ class TestCompleteStreamToolCalls:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=False,
should_persist=False,
)
)
thought_chunks = [s for s in stream if '"type": "thought"' in s]
@@ -159,7 +159,7 @@ class TestCompleteStreamStructuredOutput:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=False,
should_persist=False,
)
)
structured_chunks = [
@@ -192,7 +192,7 @@ class TestCompleteStreamMetadata:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=False,
should_persist=False,
)
)
# Should not crash, metadata handled silently
@@ -225,7 +225,7 @@ class TestCompleteStreamIsNoneDoc:
user_api_key=None,
decoded_token={"sub": "u"},
isNoneDoc=True,
should_save_conversation=False,
should_persist=False,
)
)
# Verify stream completes without error
@@ -256,7 +256,7 @@ class TestCompleteStreamErrorType:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=False,
should_persist=False,
)
)
error_chunks = [s for s in stream if '"type": "error"' in s]
@@ -281,7 +281,7 @@ class TestCompleteStreamErrorType:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=False,
should_persist=False,
)
)
custom_chunks = [s for s in stream if '"type": "custom_event"' in s]
@@ -426,7 +426,7 @@ class TestCompleteStreamCompressionMetadata:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
)
@@ -470,7 +470,7 @@ class TestCompleteStreamCompressionMetadata:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
)
@@ -501,7 +501,7 @@ class TestCompleteStreamLogTruncation:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=False,
should_persist=False,
)
)
@@ -545,7 +545,7 @@ class TestCompleteStreamGeneratorExit:
conversation_id="conv1",
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
@@ -586,7 +586,7 @@ class TestCompleteStreamGeneratorExit:
conversation_id="conv1",
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
isNoneDoc=True,
)
@@ -628,7 +628,7 @@ class TestCompleteStreamGeneratorExit:
conversation_id="conv1",
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
@@ -688,7 +688,7 @@ class TestCompleteStreamGeneratorExit:
conversation_id="conv1",
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
# Drain the early ``message_id`` event, then close before
@@ -770,7 +770,7 @@ class TestCompleteStreamGeneratorExit:
conversation_id="conv1",
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
next(gen) # message_id frame
@@ -850,7 +850,7 @@ class TestCompleteStreamGeneratorExit:
conversation_id="conv1",
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
next(gen) # message_id frame
@@ -951,7 +951,7 @@ class TestCompleteStreamWalAcceptance:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u-acceptance"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
)
@@ -1023,7 +1023,7 @@ class TestCompleteStreamWalAcceptance:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u-tap"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
)
@@ -1103,7 +1103,7 @@ class TestCompleteStreamWalAcceptance:
conversation_id=str(conv_id),
user_api_key=None,
decoded_token={"sub": user_id},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
_continuation={
"messages": [],
@@ -1153,7 +1153,7 @@ class TestCompleteStreamWalAcceptance:
conversation_id=None,
user_api_key=None,
decoded_token={"sub": "u-request-id"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
)
@@ -1215,7 +1215,7 @@ class TestStreamingHeartbeatSeed:
conversation_id="conv1",
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
)
@@ -1270,7 +1270,7 @@ class TestStreamingHeartbeatSeed:
conversation_id="conv1",
user_api_key=None,
decoded_token={"sub": "u"},
should_save_conversation=True,
should_persist=True,
model_id="gpt-4",
)
)
+18 -11
View File
@@ -248,7 +248,7 @@ test.describe('tier-a · chat turn save', () => {
}
});
test('save_conversation=false skips all four tables', async ({ browser }) => {
test('save_conversation=false persists a hidden conversation (kept out of the sidebar)', async ({ browser }) => {
const { sub, token } = await newUserContext(browser);
// Sanity: the user has nothing yet.
@@ -266,23 +266,30 @@ test.describe('tier-a · chat turn save', () => {
isNoneDoc: true,
});
expect(result.status).toBe(200);
// When save_conversation=false the route emits `{"type":"id","id":"None"}`
// (stringified None) — no real UUID persisted.
expect(result.conversationId).toBe('None');
// save_conversation now controls sidebar visibility, not persistence:
// the turn persists, so a real conversation UUID is emitted.
expect(result.conversationId).toMatch(
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i,
);
// Key invariant: conversations + conversation_messages must be empty
// for this user. token_usage still fires (it's per-LLM-call, not per
// conversation); user_logs also fires (audit log). We only assert on
// the two "persistent chat state" tables.
// The conversation + its messages persist...
expect(
await countRows('conversations', {
sql: 'user_id = $1',
params: [sub],
}),
).toBe(0);
).toBe(1);
const msgs = await fetchMessagesForUser(sub);
expect(msgs).toHaveLength(0);
expect(msgs.length).toBeGreaterThan(0);
// ...but it is hidden, so it never surfaces in the sidebar
// (list_for_user filters visibility = 'listed').
expect(
await countRows('conversations', {
sql: "user_id = $1 AND visibility = 'listed'",
params: [sub],
}),
).toBe(0);
});
test('silent-break: two concurrent /stream calls on the same conversation_id — neither 500s and positions stay unique', async ({
+52 -37
View File
@@ -1,27 +1,26 @@
/**
* Tier-B · agent preview does NOT persist conversations
* Tier-B · agent preview persists a HIDDEN conversation
*
* Covers B14. The preview flow in `frontend/src/agents/agentPreviewSlice.ts`
* calls `handleFetchAnswerSteaming(..., save_conversation=false)` — see the
* `false` positional arg at line 126. That flag flows through
* `POST /stream` (application/api/answer/routes/stream.py:55-59,142) into
* `BaseAnswerResource.complete_stream(should_save_conversation=...)`
* (application/api/answer/routes/base.py:174,290,389,479), where the
* branches that insert `conversations` / `conversation_messages` rows are
* gated on `should_save_conversation`.
* calls `handleFetchAnswerSteaming(..., save_conversation=false)`. As of the
* conversation-visibility change, `save_conversation` no longer gates
* persistence — it controls sidebar visibility only. A preview turn therefore
* persists a `conversations` row with `visibility = 'hidden'` (decided by
* `resolve_persistence` in
* application/api/answer/services/persistence_policy.py and threaded through
* `BaseAnswerResource.complete_stream(visibility=...)`).
*
* The load-bearing assertion is: after a preview stream completes, the
* `conversations` table still has zero rows for this user. If a
* regression drops the `save_conversation` flag (or flips the default),
* preview traffic would silently pollute the user's sidebar history.
* The load-bearing assertion is: after a preview stream completes, the row
* exists but is `hidden`, so it is excluded from the sidebar query
* (`ConversationsRepository.list_for_user`, which filters `visibility =
* 'listed'`). If a regression flips the preview default to `listed`, preview
* traffic would pollute the user's sidebar history.
*
* API-driven: the frontend preview UI ultimately POSTs /stream with
* `save_conversation:false` and `conversation_id:null`; we exercise that
* contract directly to keep the spec fast and isolated from the
* AgentPreview modal's rendering quirks. A standard chat POST
* (save_conversation defaulted to true) is included as a control — it
* MUST write a conversation row, so "preview wrote zero" is a meaningful
* contrast rather than "nothing ever writes".
* contract directly to keep the spec fast and isolated from the AgentPreview
* modal's rendering quirks. A standard chat POST (save_conversation omitted)
* is the control — it writes a `listed` row that DOES surface in the sidebar.
*/
import * as playwright from '@playwright/test';
@@ -58,6 +57,16 @@ async function countUserConversations(userSub: string): Promise<number> {
return Number(rows[0]?.n ?? 0);
}
/** Conversations that surface in the sidebar (the `list_for_user` filter). */
async function countListedConversations(userSub: string): Promise<number> {
const { rows } = await pg.query<{ n: string }>(
`SELECT count(*)::text AS n FROM conversations
WHERE user_id = $1 AND visibility = 'listed'`,
[userSub],
);
return Number(rows[0]?.n ?? 0);
}
async function countConversationMessages(userSub: string): Promise<number> {
const { rows } = await pg.query<{ n: string }>(
`SELECT count(*)::text AS n
@@ -71,10 +80,9 @@ async function countConversationMessages(userSub: string): Promise<number> {
/**
* POST /stream with save_conversation=false and drain the SSE body. The
* stream emits `{type:"id", id:"None"}` in preview mode (preview has no
* conversation_id to echo back) — we can't use `streamOnce` here because
* that helper insists on a UUID-shaped id. Instead, just drain the
* response and assert 200.
* stream emits `{type:"id", id:"<uuid>"}` once the (hidden) conversation is
* persisted. Sidebar membership is asserted via the DB helpers below rather
* than the stream id, so we just drain the response and assert 200.
*/
async function drainStream(
api: import('@playwright/test').APIRequestContext,
@@ -96,7 +104,7 @@ test.describe('tier-b · agent preview', () => {
await resetDb();
});
test('preview stream with save_conversation=false writes NO conversations row', async ({
test('preview stream with save_conversation=false persists a hidden conversation excluded from the sidebar', async ({
browser,
}) => {
const { context, sub, token } = await newUserContext(browser);
@@ -118,14 +126,19 @@ test.describe('tier-b · agent preview', () => {
chunks: '0',
});
// Core assertion: preview MUST NOT persist to conversations.
// The turn persists (a row + messages exist)...
expect(
await countUserConversations(sub),
'preview stream must not write a conversations row',
).toBe(0);
'preview stream persists a conversation row',
).toBe(1);
expect(
await countConversationMessages(sub),
'preview stream must not write conversation_messages rows',
'preview stream persists conversation_messages rows',
).toBeGreaterThan(0);
// ...but it is hidden, so the sidebar query returns nothing.
expect(
await countListedConversations(sub),
'preview conversation must not surface in the sidebar',
).toBe(0);
} finally {
await api.dispose();
@@ -133,7 +146,7 @@ test.describe('tier-b · agent preview', () => {
}
});
test('control: a non-preview stream (save_conversation omitted) DOES write a conversations row', async ({
test('control: a non-preview stream (save_conversation omitted) writes a LISTED conversations row', async ({
browser,
}) => {
const { context, sub, token } = await newUserContext(browser);
@@ -142,10 +155,10 @@ test.describe('tier-b · agent preview', () => {
const agentId = await createDraftAgent(sub, 'non-preview-target');
expect(await countUserConversations(sub)).toBe(0);
// Standard chat path — save_conversation defaults to true at the
// backend (stream_model field default + get() fallback in stream.py).
// This is the "control" that makes the preview assertion meaningful:
// without it, a spec where nothing ever writes could pass trivially.
// Standard chat path — save_conversation omitted, so a first-party
// interactive turn defaults to `listed` (resolve_persistence). This is
// the "control" that makes the preview assertion meaningful: without
// it, a spec where nothing ever surfaces could pass trivially.
const conversationId = await streamOnce(api, {
question: 'Hello persistence',
conversation_id: null,
@@ -157,8 +170,9 @@ test.describe('tier-b · agent preview', () => {
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i,
);
// Exactly one conversation row landed.
// Exactly one conversation row landed, and it surfaces in the sidebar.
expect(await countUserConversations(sub)).toBe(1);
expect(await countListedConversations(sub)).toBe(1);
expect(await countConversationMessages(sub)).toBeGreaterThan(0);
} finally {
await api.dispose();
@@ -166,7 +180,7 @@ test.describe('tier-b · agent preview', () => {
}
});
test('multiple preview turns still produce zero conversations rows', async ({
test('multiple preview turns persist hidden rows that never surface in the sidebar', async ({
browser,
}) => {
const { context, sub, token } = await newUserContext(browser);
@@ -186,10 +200,11 @@ test.describe('tier-b · agent preview', () => {
});
}
// Still zero — no preview turn ever persists, regardless of how
// many times the user clicks "Test message".
expect(await countUserConversations(sub)).toBe(0);
expect(await countConversationMessages(sub)).toBe(0);
// Each turn persisted its own hidden conversation...
expect(await countUserConversations(sub)).toBe(3);
// ...yet the sidebar stays empty no matter how many "Test message"
// clicks the user makes.
expect(await countListedConversations(sub)).toBe(0);
} finally {
await api.dispose();
await context.close();
@@ -75,14 +75,28 @@ class TestListForUser:
assert len(results) == 2
assert all(r["user_id"] == "alice" for r in results)
def test_excludes_api_key_without_agent(self, pg_conn):
def test_excludes_hidden_conversations(self, pg_conn):
repo = _repo(pg_conn)
repo.create("alice", "normal")
repo.create("alice", "api-only", api_key="key-1")
repo.create("alice", "hidden-one", visibility="hidden")
results = repo.list_for_user("alice")
assert len(results) == 1
assert results[0]["name"] == "normal"
def test_lists_api_key_conversation_when_listed(self, pg_conn):
# Visibility is explicit now: an api_key conversation created as
# ``listed`` surfaces (the old api_key/agent_id heuristic is gone).
repo = _repo(pg_conn)
repo.create("alice", "api-listed", api_key="key-1", visibility="listed")
results = repo.list_for_user("alice")
assert len(results) == 1
assert results[0]["name"] == "api-listed"
def test_create_defaults_to_listed(self, pg_conn):
repo = _repo(pg_conn)
conv = repo.create("alice", "default-vis")
assert conv["visibility"] == "listed"
class TestRename:
def test_renames(self, pg_conn):
+22 -3
View File
@@ -214,7 +214,8 @@ class TestTranslateRequest:
result = translate_request(data, "test-key")
assert result["question"] == "What's 2+2?"
assert result["api_key"] == "test-key"
# Conversations are not persisted by default on the v1 endpoint.
# v1 conversations persist; ``save_conversation`` only controls whether
# they list in the sidebar (default off).
assert result["save_conversation"] is False
history = json.loads(result["history"])
assert len(history) == 1
@@ -265,7 +266,7 @@ class TestTranslateRequest:
],
}
result = translate_request(data, "key")
assert result["save_conversation"] is True
assert result["persist"] is True
def test_stateless_continuation_does_not_persist_by_default(self):
"""A stateless continuation (no conversation_id, e.g. an OpenAI client
@@ -282,7 +283,25 @@ class TestTranslateRequest:
],
}
result = translate_request(data, "key")
assert result["save_conversation"] is False
assert result["persist"] is False
def test_stateful_continuation_persist_override_via_docsgpt(self):
"""``docsgpt.persist=false`` forces no persistence even with a
conversation_id."""
data = {
"conversation_id": "conv-1",
"docsgpt": {"persist": False},
"messages": [
{"role": "user", "content": "Search for X"},
{
"role": "assistant",
"tool_calls": [{"id": "c1", "type": "function", "function": {"name": "search", "arguments": "{}"}}],
},
{"role": "tool", "tool_call_id": "c1", "content": "done"},
],
}
result = translate_request(data, "key")
assert result["persist"] is False
def test_continuation_honours_explicit_save_conversation_override(self):
"""An explicit docsgpt.save_conversation=false on the continuation wins."""