Skip to content

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

Merged
asukaminato0721 merged 1 commit into
langgenius:mainfrom
Kiveiru3:refactor/40372-conversation-session-accessors
Sep 7, 2026
Merged

asukaminato0721 merged 1 commit into
langgenius:mainfrom
Kiveiru3:refactor/40372-conversation-session-accessors

Conversation

@Kiveiru3

@Kiveiru3 Kiveiru3 commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Part of #40372, following the established model cleanup pattern from #40370, #41569, and #41721.

Removes ten legacy @Property accessors on Conversation in api/models/model.py that implicitly reach for the global db.session():

  • summary_or_query
  • annotated
  • annotation
  • message_count
  • user_feedback_stats
  • admin_feedback_stats
  • status_count
  • first_message
  • from_end_user_session_id
  • from_account_name

All production response reads already go through ConversationResponseSource and the existing *_with_session methods. The direct model and container-test reads were updated to pass the caller-owned session explicitly.

This intentionally leaves inputs, model_config, and app untouched because they still have active property consumers or setter behavior and require a separate migration.

Claimed this slice in #40372 (comment).

Tests

  • pytest -q api/tests/unit_tests/models/test_app_models.py api/tests/unit_tests/controllers/console/app/test_conversation_api.py — 61 passed
  • Ruff check passed
  • Ruff format check passed
  • Pyrefly passed for api/models/model.py
  • Mypy passed for api/models/model.py

Screenshots

N/A (backend-only refactor).

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran make lint && make type-check (backend) and vp staged (frontend) to appease the lint gods

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-09-05 10:30:55.930331003 +0000
+++ /tmp/pyrefly_pr.txt	2026-09-05 10:30:47.398343503 +0000
@@ -8075,7 +8075,7 @@
 ERROR Argument `dict[str, bool]` is not assignable to parameter `annotation_reply` with type `AnnotationReplyDisabledConfig | AnnotationReplyEnabledConfig | None` in function `models.model.AppModelConfig.to_dict` [bad-argument-type]
    --> tests/unit_tests/models/test_app_models.py:398:54
 ERROR Object of class `FromClause` has no attribute `insert` [missing-attribute]
-    --> tests/unit_tests/models/test_app_models.py:1132:13
+    --> tests/unit_tests/models/test_app_models.py:1133:13
 ERROR `None` is not subscriptable [unsupported-operation]
    --> tests/unit_tests/models/test_dataset_models.py:331:16
 ERROR `None` is not subscriptable [unsupported-operation]

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 63.14% 63.14% -0.00%
Strict coverage 62.75% 62.75% -0.00%
Typed symbols 44,671 44,664 -7
Untyped symbols 26,242 26,240 -2
Modules 3344 3344 0

@asukaminato0721
asukaminato0721 added this pull request to the merge queue Sep 7, 2026
Merged via the queue into langgenius:main with commit e7d34f0 Sep 7, 2026
47 checks passed
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.

2 participants