rewrite property to accept session: Session as parameter

@dosu

example: #40370

@dosu show me the file list to clean and what to do.

Here's the full inventory of files and what needs to be done. The pattern from PR #40370 is: remove @property, add session: Session parameter, and replace internal db.session() usage.

Files to clean (~113 properties/methods across 9 files)

1. api/models/model.py — ~47 items [1]

37 properties with _with_session helpers — e.g. desc_or_prompt, site, app_model_config, workflow, bound_agent_id, is_agent, tags, author_name, inputs, model_config, 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, user_feedback, admin_feedback, feedbacks, agent_thoughts, message_files, from_account, etc.
10 properties directly using db.session — e.g. tenant, annotation_reply_dict, app (RecommendedApp, ApiToken), user (ApiToken), account (OperationLog), collection_binding_detail, generate_unique_server_code

2. api/models/dataset.py — ~35 items

26 properties with helpers — total_documents, total_available_documents, created_by_account, author_name, latest_process_rule, app_count, document_count, external_knowledge_info, is_published, doc_metadata, data_source_detail_dict, dataset_process_rule, dataset, segment_count, hit_count, uploader, doc_metadata_details, child_chunks, attachments, queries, dataset_bindings, etc.
9 properties directly using db.session — available_document_count, available_segment_count, previous_segment, next_segment, dataset/document/segment (DocumentKeyword), app (AppDatasetJoin), created_user_name
⚠️ word_count, doc_form, tags already handled in chore: example to rm db.session #40370

3. api/models/workflow.py — ~12 items

3 properties with helpers: created_by_account, updated_by_account, tool_published
9 direct usages including deprecated ones: created_by_account/created_by_end_user (WorkflowRun, WorkflowNodeExecution, WorkflowAppLog), message, workflow, _resolve_workflow_app_tenant_id

4. api/models/snippet.py — 4 items

graph, tags, created_by_account, updated_by_account

5. api/models/comment.py — 4 items

created_by_account (CommentThread, Comment), resolved_by_account, mentioned_user_account

6. api/models/tools.py — 6 items

user/tenant (ApiToolProvider, WorkflowToolProvider), app (WorkflowToolProvider), load_user (MCPToolProvider)

7. api/models/trigger.py — 2 items

created_by_account, created_by_end_user (WorkflowTriggerLog)

8. api/models/account.py — 2 items

get_by_openid classmethod uses db.session.execute() / db.session.scalar()

9. api/models/web.py — 1 item

message (SavedMessage)

What to do for each

For properties WITH existing get_* / *_with_session helpers:

Remove @property
Add session: Session parameter
Forward to the helper (remove session=db.session() call)
Remove * keyword-only marker from the helper if present

For properties WITHOUT helpers (direct db.session usage):

Remove @property
Add session: Session parameter
Replace db.session / db.session() with the session parameter
Update all call sites to pass session explicitly

This is a large effort (~113 items). I'd suggest tackling it file-by-file, starting with the biggest wins (model.py and dataset.py).

To reply, just mention @dosu.

Docs are dead. Just use Dosu.

I'd like to work on this as a follow-up to #40370. Opening a PR that converts the remaining @property accessors on models/web.py:SavedMessage and models/trigger.py:WorkflowTriggerLog that internally call db.session... into plain methods taking session: Session explicitly, matching the pattern from #40370:

SavedMessage.message
WorkflowTriggerLog.created_by_account
WorkflowTriggerLog.created_by_end_user

Confirmed (via repo-wide grep) there are no other call sites of these three accessors outside their own model files and tests, so this is a self-contained, non-breaking change. Will link the PR here once it's up.

Note: there are other @property-with-db.session accessors elsewhere in the codebase (e.g. models/comment.py, models/dataset.py, models/workflow.py — some already partially migrated by #40370/#40379/#40453) that are out of scope for this PR; happy to pick up further batches in follow-ups if useful.

Hi, I'd like to work on the api/models/account.py part of this issue.

I'll refactor Account.get_by_openid to accept a caller-provided Session instead of accessing db.session internally, and update all related call sites and tests.

I'll keep the scope focused on account.py to avoid overlapping with the ongoing web.py / trigger.py work.

Update: I found that #40453 already covers the api/models/account.py scope, so I won't duplicate that work.

I'd like to take the api/models/comment.py part instead, refactoring the remaining account-related properties that access db.session internally to accept an explicit Session, and updating the related call sites and tests.

I'll keep the PR scoped to comment.py and its required callers.

refactor(models): dep-inject Session on @property-dead accessors

I'd like to work on the api/models/tools.py part of this issue. I'll refactor the 6 @property accessors that internally call db.session (ApiToolProvider.user, ApiToolProvider.tenant, WorkflowToolProvider.user, WorkflowToolProvider.tenant, WorkflowToolProvider.app, MCPToolProvider.load_user) into plain methods taking session: Session explicitly, matching the pattern from #40370. Will link the PR here once it's up.

Hi, I'd like to take a narrow, non-overlapping slice in api/models/dataset.py: DocumentSegment.previous_segment and DocumentSegment.next_segment.

On current main (18d3990), both are still @property accessors that call the global db.session. GitHub code search finds only their definitions and the two existing container integration tests. I would convert only these accessors into methods accepting a caller-owned Session, switch the queries to session.scalar(...), and update those focused tests to pass db_session_with_containers. I found no open PR mentioning either symbol, and this does not overlap the snippet.py, account.py, comment.py, tools.py, web.py, or trigger.py slices already discussed here.

Could a maintainer assign this slice before I start implementation, as required by the PR template? I am using Codex for automated assistance, and any resulting PR description will end with From Codex.

Hi, I'd like to take a narrow, non-overlapping slice in
api/models/workflow.py:

Workflow.created_by_account
Workflow.updated_by_account
Workflow.tool_published

On current main (8abff1b), these are still @property wrappers that
obtain db.session() and forward it to existing get_* helpers.

I propose to:

convert the three wrappers into methods that require a caller-provided
orm.Session;
forward the supplied session and remove the helpers' keyword-only marker,
matching the pattern established in chore: example to rm db.session #40370;
update focused tests in
api/tests/unit_tests/models/test_workflow.py.

The current controller adapters already call the get_* helpers with an
owned session, and I found no open PR mentioning these symbols. This scope
does not overlap with the snippet.py, account.py, comment.py,
tools.py, web.py / trigger.py, or newly requested dataset.py slices.

Could a maintainer assign this slice to me before I start implementation,
as required by the PR template?

I will use Codex for assistance and will end the resulting PR description
with From Codex.

Hi, I'd like to take a narrow, non-overlapping slice in api/models/model.py: the five annotation/feedback Account accessors.

MessageFeedback.from_account
MessageAnnotation.account
MessageAnnotation.annotation_create_account
AppAnnotationHitHistory.account
AppAnnotationHitHistory.annotation_create_account

On current main (ac23430), the first three are @property wrappers that call db.session() and forward to an existing *_with_session helper; the last two still call the global db.session directly and have no helper at all.

I propose to:

collapse each property and its *_with_session helper into a single method taking a caller-provided session: Session, dropping the keyword-only marker, matching the pattern from chore: example to rm db.session #40370;
give the two AppAnnotationHitHistory accessors the same explicit-session signature and a proper Account | None return annotation (they are currently untyped);
update the three _FeedbackResponseSource / _AnnotationResponseSource adapters in api/fields/conversation_fields.py and the focused tests in api/tests/unit_tests/controllers/console/app/test_message_api.py.

I verified with a repo-wide grep that every serialization path for these five reaches them through the MessageResponseSource adapters in fields/conversation_fields.py, which already own a session — so the @property forms are effectively dead and no controller or response model reads them by attribute name. AnnotationHitHistory in fields/annotation_fields.py exposes no account field at all.

This slice is inside api/models/model.py, which I could not find claimed anywhere in this thread, and does not overlap the snippet.py, account.py (#40453), comment.py, tools.py, web.py / trigger.py, dataset.py (DocumentSegment), or workflow.py slices already discussed here.

Could a maintainer assign this slice before I open the PR, as required by the PR template?

Hi, I'd like to take a narrow, non-overlapping slice in api/models/dataset.py: the three ChildChunk accessors.

ChildChunk.dataset
ChildChunk.document
ChildChunk.segment

On current main they are still @property accessors that call the global db.session internally. I'll convert them to methods accepting a caller-provided Session, following the pattern in #40370, and update the call sites and tests accordingly.

Hi, I would like to take a narrow, non-overlapping slice in api/models/dataset.py: PipelineCustomizedTemplate.created_user_name.

On current main (54298ab), it is still an @property that reads from the global db.session. Its only production caller is CustomizedPipelineTemplateRetrieval.fetch_pipeline_template_detail_from_db, which already owns a Session; the focused unit test currently monkeypatches models.dataset.db solely for this lookup.

I propose to convert it to a method accepting a caller-provided Session, pass the existing session at the sole call site, and update the focused SQLite-backed test to remove the global-session monkeypatch. I found no open PR or comment claiming this symbol, and it does not overlap the currently claimed dataset.py slices (DocumentSegment.previous_segment/next_segment and ChildChunk.dataset/document/segment).

Could a maintainer confirm and assign this slice before I start, as required by the PR template?

I am using Codex for assistance and will disclose it in the PR description.

Update: I am withdrawing this claim so that I do not reserve the slice while waiting for assignment. I found a separate contribution to work on. Please consider PipelineCustomizedTemplate.created_user_name available to other contributors.

Picking up the slice RealJasonHu withdrew: api/models/dataset.py: PipelineCustomizedTemplate.created_user_name.

It's still an @property reading from the global db.session. The only production caller (CustomizedPipelineTemplateRetrieval.fetch_pipeline_template_detail_from_db in services/rag_pipeline/pipeline_template/customized/customized_retrieval.py) already owns a Session, so this is a self-contained change: convert to a method taking session: Session explicitly, update that one call site, and drop the monkeypatch.setattr("models.dataset.db", ...) hack in the existing unit test in favor of passing the real session, matching the pattern from #40370/#40797.

Will link the PR here once it's up.

Hi maintainers, just following up on the workflow.py slice proposed above.

Could you confirm whether I may proceed with these three accessors and assign
this issue to me? I have not started implementation and will wait for assignment
to avoid duplicate work.

Thanks!

Opened #42432 for the MCPToolProvider.load_user slice.

@keith991001

Hi, I'd like to take App.tenant (api/models/model.py:534) — per your @dosu query above it is one of the two symbols that were still unclaimed, and the other (MCPToolProvider.load_user) has since been taken and merged in #42432.

@property
def tenant(self) -> Tenant | None:
    return db.session.scalar(select(Tenant).where(Tenant.id == self.tenant_id))

Plan: replace the property outright with def tenant(self, session) -> Tenant | None, matching the three .tenant accessors already converted this way — InstalledApp.tenant (model.py:999, #41831), ApiToolProvider.tenant and WorkflowToolProvider.tenant (tools.py:204/280, #41830). No tenant_with_session twin: that wrapper shape was what #41831 originally proposed, and it was replaced by the plain method before approval.

One deliberate deviation from those three, which I'd like to flag up front. All three have zero production callers — they are only ever invoked from their own test files (grep -rnE "\.tenant\(" api/ --include=*.py outside tests/ returns nothing). App.tenant is the first one in this family with real readers, and they are in a controller that holds db.session, which is a scoped_session, not a Session. So the annotation has to be session: Session | scoped_session rather than the precedents' bare Session. There is already precedent for exactly that widening on this same class — App.agent_app_binding_with_session (model.py:492) and services/installed_app_service.py:107 both declare Session | scoped_session so a controller can pass db.session directly.

Readers, enumerated three independent ways (plain grep -rnE, a receiver-level ast walk, and a literal-name sweep over getattr/setattr/patch.object/AliasChoices):

Site	Today	After
`controllers/web/human_input_form.py:185`	`tenant = app_model.tenant`	`tenant = app_model.tenant(db.session)`
`controllers/web/human_input_form.py:279`	`if app_model.tenant is None or app_model.tenant.status == TenantStatus.ARCHIVE:` — two SELECTs on one line	bind once, then check

Those are the only two. The 14 .tenant hits in controllers/openapi/auth/prepare.py / verify.py are AuthData.tenant, a pydantic field declared at controllers/openapi/auth/data.py:54, not an App.

⚠️ Worth recording for anyone else auditing this family: git grep -E "\.tenant\b" returns zero hits in this repo — git's regex engine does not handle \b here. Plain grep returns 43. I nearly shipped an empty reader list off the first sweep.

This is a behaviour-affecting conversion, not a pure deletion: with the model change alone and the controller left untouched, app_model.tenant becomes a bound method, so the is None guard silently stops firing and .status then raises. I confirmed that locally — reverting only the controller half turns three existing tests in tests/unit_tests/controllers/web/test_human_input_form.py red with AttributeError: 'function' object has no attribute 'status'. Both sites are converted in the same commit.

Not touching Dataset.available_segment_count (claimed by @Ultronen above), the to_dict-blocked group or Conversation/Message inputs (claimed by @keith991001), or Account.current_tenant in account.py.

Opening the PR now — happy to close it if this overlaps with a claim I missed.

I'd like to take a narrow, non-overlapping slice in api/models/model.py:

Conversation.to_dict
Message.to_dict

On current main both still call inputs_with_session(session=db.session()) internally. I'll convert them to to_dict(self, *, session: Session) and update the production callers in clear_free_plan_tenant_expired_logs.py and ops_trace_manager.py (including the adjacent inputs_with_session(session=db.session()) on the dataset-retrieval trace path). Focused unit test to follow.

This does not overlap open PRs #41531, or the remaining AppModelConfig.annotation_reply_dict slice.

I'd like to take Conversation.app in api/models/model.py. On current main it is a @property that opens its own Session(db.engine, expire_on_commit=False). Its only production callers are the two reads in core/memory/token_buffer_memory.py.

Plan: turn it into app(session) and pass the db.session() that TokenBufferMemory already uses for the adjacent model_config_with_session lookup. Tests will be updated in test_token_buffer_memory.py and test_app_models.py. This doesn't touch Provider.credential, ProviderModel.credential or AppModelConfig.annotation_reply_dict. PR to follow.

@dosu

@dosu show me the file list to clean and what to do.

With App.tenant and MCPToolProvider.load_user claimed or merged, two direct db.session usages remain in api/models/ on current main [1] [2]. dataset.py, tools.py, account.py, comment.py, snippet.py, web.py and trigger.py no longer make raw db.session calls.

api/models/model.py: AppModelConfig.annotation_reply_dict (L811-812)

This is small, self-contained and ready to claim. It still calls load_annotation_reply_config(db.session(), self.app_id) and is the last piece of the to_dict-blocked group you approved. It has real readers via AliasChoices("annotation_reply_dict", "annotation_reply") in AppModelConfigResponse [3] and in trial.py's response model. Both already resolve through AppModelConfigResponseView, a session-carrying wrapper in services/app_service.py whose own annotation_reply_dict property forwards to self._session [4].

Converting the model property to a plain annotation_reply_dict(self, *, session: Session) method therefore needs no controller changes:

Drop the property.
Update the container-integration test test_annotation_reply_dict_disabled_without_setting.
Update the unit test in test_app_models.py that patches it as a PropertyMock.

api/models/workflow.py: _resolve_workflow_app_tenant_id (L92-98)

This one needs someone to map the call sites before it counts as a narrow slice. It is a module-level function, not a property, and still calls db.session.scalar(...). WorkflowDraftVariable._rebuild_file_types uses it twice (L1734, L1744) to resolve a file's tenant for reconstruction. Converting it means threading a session parameter through _rebuild_file_types and its two callers in build_segment_from_serialized_value [5]. Other WorkflowDraftVariable methods call that function, so the whole chain needs auditing.

To reply, just mention @dosu.

Docs are dead. Just use Dosu.

I'd like to take AppModelConfig.annotation_reply_dict (api/models/model.py L811).

Scope:

drop the property and convert it to annotation_reply_dict(self, *, session: Session)
handle the to_dict fallback at L891 (annotation_reply=None currently reads the property) after auditing its callers
update the container-integration test test_annotation_reply_dict_disabled_without_setting and the PropertyMock unit test in test_app_models.py

No controller changes expected — the pydantic readers already resolve through the session-carrying AppModelConfigResponseView.

I'd like to take the last remaining item from the list above: _resolve_workflow_app_tenant_id (api/models/workflow.py L92).

Mapped call sites, so the scope is:

convert _resolve_workflow_app_tenant_id to take an explicit session: Session
thread the session through its only consumer chain: WorkflowDraftVariable._rebuild_file_types → build_segment_from_serialized_value → _loads_value → get_value()
callers: 4 sites in WorkflowDraftVariableService (already holds _session) + the direct build_segment_from_serialized_value call in the offloaded-file path
the implicit reader in fields/workflow_draft_variable_fields.py (_serialize_var_value via dump_response) gets a session-carrying response source on the shared _SessionResponseSource base from refactor(models): pass session into WorkflowRun accessors and drop dead ones #42034, so controllers keep their existing @with_session wiring
get_value()'s caching contract is preserved: first call decodes with the caller's session, later calls hit the cache

From Claude Code

@dosu

@dosu any files left?

@keith991001

Yes: 3 files still contain implicit database access in the current checkout (34c9438623), including properties that create their own sessions rather than use db.session.

File	Remaining entry points	What to do
`api/models/workflow.py`	`_resolve_workflow_app_tenant_id`	Still calls `db.session.scalar(...)`. Thread the caller's session through the draft-variable decoding chain. Already claimed by @keith991001 in the latest comment. [1]
`api/models/workflow.py`	`WorkflowAppLog.workflow_run`	Property constructs a session factory and queries through a repository. Replace implicit session ownership with an explicit-session lookup, auditing callers and response serialization. [2]
`api/models/model.py`	`Message.workflow_run`	Same session-factory/repository pattern. Convert alongside the log accessor or as a separate focused slice. [3]
`api/models/provider.py`	`Provider.credential`, `ProviderModel.credential`	Both are `cached_property` accessors that open sessions through `session_factory.create_session()`. Convert to explicit-session methods and audit dependent `credential_name` / `encrypted_config` properties, including the provider's `token_is_set` / `is_enabled` chain. Decide how to preserve caching semantics. [4] [5]

The previous AppModelConfig.annotation_reply_dict item is gone: AppModelConfig.to_dict() now requires an explicit annotation_reply value, with no implicit fallback. [6]

So the additional cleanup candidates beyond Keith's claimed resolver are four accessors across these three files. Their session creation was missed by the earlier db.session-only inventory.

To reply, just mention @dosu.

Docs are dead. Just use Dosu.

#45166

Navigation Menu

Repository navigation

rewrite property to accept session: Session as parameter #40372

Files to clean (~113 properties/methods across 9 files)

What to do for each

40 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

rewrite property to accept session: Session as parameter #40372

Description

Activity

dosubot commented on Aug 9, 2026

Files to clean (~113 properties/methods across 9 files)

What to do for each

a-yeyang commented on Aug 14, 2026

ddddajie commented on Aug 17, 2026

ddddajie commented on Aug 17, 2026

morning-verlu commented on Aug 18, 2026

dvd233 commented on Aug 25, 2026

jjj-n commented on Aug 25, 2026

hur26 commented on Aug 27, 2026

keith991001 commented on Aug 27, 2026

RealJasonHu commented on Aug 27, 2026

RealJasonHu commented on Aug 27, 2026

a-yeyang commented on Aug 27, 2026

jjj-n commented on Aug 28, 2026

40 remaining items

dorian0326 commented on Sep 17, 2026

JessYanCoding commented on Sep 18, 2026

dorian0326 commented on Sep 23, 2026

destire-mio commented on Sep 26, 2026

asukaminato0721 commented on Sep 28, 2026

dosubot commented on Sep 28, 2026

keith991001 commented on Sep 28, 2026

keith991001 commented on Sep 29, 2026

asukaminato0721 commented on Oct 30, 2026

dosubot commented on Oct 30, 2026

asukaminato0721 commented on Oct 30, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions