Skip to content

refactor(models): remove legacy db.session properties from Conversation #41847

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.

Sign up for GitHub

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

Merged
asukaminato0721 merged 1 commit into from
Sep 7, 2026
Merged

refactor(models): remove legacy db.session properties from Conversation #41847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 0 additions & 40 deletions api/models/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1333,10 +1333,6 @@ def model_config_with_session(self, *, session: Session) -> AppModelConfigDict:

return model_config

@property
def summary_or_query(self):
return self.summary_or_query_with_session(session=db.session())

def summary_or_query_with_session(self, *, session: Session) -> str:
if self.summary:
return self.summary
Expand All @@ -1347,41 +1343,21 @@ def summary_or_query_with_session(self, *, session: Session) -> str:
else:
return ""

@property
def annotated(self) -> bool:
return self.annotated_with_session(session=db.session())

def annotated_with_session(self, *, session: Session) -> bool:
return (
session.scalar(select(func.count(MessageAnnotation.id)).where(MessageAnnotation.conversation_id == self.id))
or 0
) > 0

@property
def annotation(self) -> MessageAnnotation | None:
return self.annotation_with_session(session=db.session())

def annotation_with_session(self, *, session: Session) -> MessageAnnotation | None:
return session.scalar(select(MessageAnnotation).where(MessageAnnotation.conversation_id == self.id).limit(1))

@property
def message_count(self) -> int:
return self.message_count_with_session(session=db.session())

def message_count_with_session(self, *, session: Session) -> int:
return session.scalar(select(func.count(Message.id)).where(Message.conversation_id == self.id)) or 0

@property
def user_feedback_stats(self) -> dict[str, int]:
return self.user_feedback_stats_with_session(session=db.session())

def user_feedback_stats_with_session(self, *, session: Session) -> dict[str, int]:
return self._feedback_stats_with_session(session=session, from_source=FeedbackFromSource.USER)

@property
def admin_feedback_stats(self) -> dict[str, int]:
return self.admin_feedback_stats_with_session(session=db.session())

def admin_feedback_stats_with_session(self, *, session: Session) -> dict[str, int]:
return self._feedback_stats_with_session(session=session, from_source=FeedbackFromSource.ADMIN)

Expand Down Expand Up @@ -1410,10 +1386,6 @@ def _feedback_stats_with_session(self, *, session: Session, from_source: Feedbac

return {"like": like, "dislike": dislike}

@property
def status_count(self):
return self.status_count_with_session(session=db.session())

def status_count_with_session(self, *, session: Session) -> dict[str, int] | None:
from models.workflow import WorkflowRun

Expand Down Expand Up @@ -1468,10 +1440,6 @@ def status_count_with_session(self, *, session: Session) -> dict[str, int] | Non
"paused": status_counts[WorkflowExecutionStatus.PAUSED],
}

@property
def first_message(self) -> Message | None:
return self.first_message_with_session(session=db.session())

def first_message_with_session(self, *, session: Session) -> Message | None:
return session.scalar(
select(Message).where(Message.conversation_id == self.id).order_by(Message.created_at.asc())
Expand All @@ -1482,10 +1450,6 @@ def app(self) -> App | None:
with Session(db.engine, expire_on_commit=False) as session:
return session.scalar(select(App).where(App.id == self.app_id))

@property
def from_end_user_session_id(self) -> str | None:
return self.from_end_user_session_id_with_session(session=db.session())

def from_end_user_session_id_with_session(self, *, session: Session) -> str | None:
if self.from_end_user_id:
end_user = session.scalar(select(EndUser).where(EndUser.id == self.from_end_user_id))
Expand All @@ -1494,10 +1458,6 @@ def from_end_user_session_id_with_session(self, *, session: Session) -> str | No

return None

@property
def from_account_name(self) -> str | None:
return self.from_account_name_with_session(session=db.session())

def from_account_name_with_session(self, *, session: Session) -> str | None:
if self.from_account_id:
account = session.scalar(select(Account).where(Account.id == self.from_account_id))
Expand Down
18 changes: 9 additions & 9 deletions api/tests/test_containers_integration_tests/models/test_conversation_status_count.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
Integration tests for Conversation.status_count and Site.generate_code model properties.
Integration tests for Conversation.status_count_with_session and Site.generate_code.

Migrated from unit_tests/models/test_app_models.py TestConversationStatusCount and
test_site_generate_code, replacing db.session.scalars mocks with real PostgreSQL queries.
Expand All @@ -18,7 +18,7 @@


class TestConversationStatusCount:
"""Integration tests for Conversation.status_count property."""
"""Integration tests for Conversation.status_count_with_session."""

@pytest.fixture(autouse=True)
def _auto_rollback(self, db_session_with_containers: Session) -> Generator[None, None, None]:
Expand Down Expand Up @@ -130,7 +130,7 @@ def test_status_count_returns_none_when_no_messages(self, db_session_with_contai
app = self._create_app(db_session_with_containers, tenant_id, created_by)
conversation = self._create_conversation(db_session_with_containers, app)

result = conversation.status_count
result = conversation.status_count_with_session(session=db_session_with_containers)

assert result is None

Expand All @@ -145,7 +145,7 @@ def test_status_count_returns_none_when_messages_have_no_workflow_run_id(
conversation = self._create_conversation(db_session_with_containers, app)
self._create_message(db_session_with_containers, app, conversation, workflow_run_id=None)

result = conversation.status_count
result = conversation.status_count_with_session(session=db_session_with_containers)

assert result is None

Expand All @@ -162,7 +162,7 @@ def test_status_count_counts_succeeded_workflow_run(self, db_session_with_contai
)
self._create_message(db_session_with_containers, app, conversation, workflow_run_id=run.id)

result = conversation.status_count
result = conversation.status_count_with_session(session=db_session_with_containers)

assert result is not None
assert result["success"] == 1
Expand All @@ -183,7 +183,7 @@ def test_status_count_counts_failed_workflow_run(self, db_session_with_container
)
self._create_message(db_session_with_containers, app, conversation, workflow_run_id=run.id)

result = conversation.status_count
result = conversation.status_count_with_session(session=db_session_with_containers)

assert result is not None
assert result["success"] == 0
Expand All @@ -204,7 +204,7 @@ def test_status_count_counts_paused_workflow_run(self, db_session_with_container
)
self._create_message(db_session_with_containers, app, conversation, workflow_run_id=run.id)

result = conversation.status_count
result = conversation.status_count_with_session(session=db_session_with_containers)

assert result is not None
assert result["success"] == 0
Expand All @@ -230,7 +230,7 @@ def test_status_count_multiple_statuses(self, db_session_with_containers: Sessio
run = self._create_workflow_run(db_session_with_containers, app, workflow, status, created_by)
self._create_message(db_session_with_containers, app, conversation, workflow_run_id=run.id)

result = conversation.status_count
result = conversation.status_count_with_session(session=db_session_with_containers)

assert result is not None
assert result["success"] == 1
Expand All @@ -255,7 +255,7 @@ def test_status_count_filters_workflow_runs_by_app_id(self, db_session_with_cont
# Message references that run but is in a conversation under app
self._create_message(db_session_with_containers, app, conversation, workflow_run_id=other_run.id)

result = conversation.status_count
result = conversation.status_count_with_session(session=db_session_with_containers)

# The run should be excluded because app_id filter doesn't match
assert result is not None
Expand Down
9 changes: 5 additions & 4 deletions api/tests/unit_tests/models/test_app_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,8 +541,9 @@ def test_conversation_inputs_setter(self):
# Assert
assert conversation._inputs == inputs

def test_conversation_summary_or_query_with_summary(self):
"""Test summary_or_query property when summary exists."""
@pytest.mark.parametrize("sqlite_session", [(Conversation, Message)], indirect=True)
def test_conversation_summary_or_query_with_summary(self, sqlite_session: Session):
"""Test summary_or_query_with_session when summary exists."""
# Arrange
conversation = Conversation(
app_id=str(uuid4()),
Expand All @@ -555,14 +556,14 @@ def test_conversation_summary_or_query_with_summary(self):
)

# Act
result = conversation.summary_or_query
result = conversation.summary_or_query_with_session(session=sqlite_session)

# Assert
assert result == "Test summary"

@pytest.mark.parametrize("sqlite_session", [(Conversation, Message)], indirect=True)
def test_conversation_summary_or_query_without_summary(self, sqlite_session: Session):
"""Test summary_or_query property when summary is empty."""
"""Test summary_or_query_with_session when summary is empty."""
# Arrange
conversation = Conversation(
app_id=str(uuid4()),
Expand Down
Loading