Skip to content

fix: correct flow operator detection in _module.py (closes #2280) - #3162

Open
botbikamordehai2-sketch wants to merge 1 commit into
eosphoros-ai:mainfrom
botbikamordehai2-sketch:fix/issue-2280-1785155282
Open

fix: correct flow operator detection in _module.py (closes #2280)#3162
botbikamordehai2-sketch wants to merge 1 commit into
eosphoros-ai:mainfrom
botbikamordehai2-sketch:fix/issue-2280-1785155282

Conversation

@botbikamordehai2-sketch

Copy link
Copy Markdown

What

The _is_flow_operator function in dbgpt/util/cli/_module.py incorrectly identified flow operators when cls.metadata was a property or descriptor rather than a direct ViewMetadata instance. This caused AWEL flow scanning to fail, leading to database schema errors (no such table: gpts_app_collection) during server startup.

Fix

Updated _is_flow_operator to safely retrieve metadata via getattr, handle callable properties, and wrap the check in a try-except for robustness. This ensures correct classification of flow operators and resources.

Closes #2280

@github-actions github-actions Bot added the fix Bug fixes label Jul 27, 2026
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary

  1. Updated _is_flow_operator to safely inspect cls.metadata, including callable properties and descriptors. Metadata access and normalization errors now return False, preventing AWEL flow scanning from aborting server startup.

  2. Changes are limited to dbgpt-core and do not alter public declarations, data, configuration, or APIs.

  3. The defensive handling improves compatibility with descriptor-backed metadata and avoids startup failures. The broad exception handling may conceal unexpected metadata errors, but no security or material performance risks are introduced.

  4. No test coverage was provided for callable, missing, or exception-raising metadata. Add focused unit tests for these cases and verify with the repository’s targeted dbgpt-core test command once confirmed.

Walkthrough

_is_flow_operator now safely resolves class metadata, supports callable metadata providers, validates BaseOperator and ViewMetadata types, and returns False when metadata is absent or access raises an exception.

Changes

Flow operator detection

Layer / File(s) Summary
Metadata normalization and validation
packages/dbgpt-core/src/dbgpt/util/cli/_module.py
_is_flow_operator resolves direct or callable metadata, validates the operator and metadata types, and handles metadata errors by returning False.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commit format and accurately describes the flow-operator fix.
Description check ✅ Passed The description includes the change summary, motivation/context, and linked issue; missing test and checklist details are non-critical.
Linked Issues check ✅ Passed The fix matches #2280 by hardening _is_flow_operator so startup no longer fails during AWEL flow scanning.
Out of Scope Changes check ✅ Passed The PR stays focused on the reported flow-operator detection bug and does not introduce unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 74e7d296-82c2-450e-9aef-57c94fb7480d

📥 Commits

Reviewing files that changed from the base of the PR and between a9e7a3c and 2b68e1f.

📒 Files selected for processing (1)
  • packages/dbgpt-core/src/dbgpt/util/cli/_module.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use Python 3.10 or newer for project development.

Files:

  • packages/dbgpt-core/src/dbgpt/util/cli/_module.py
packages/dbgpt-core/src/dbgpt/**/*.py

⚙️ CodeRabbit configuration file

packages/dbgpt-core/src/dbgpt/**/*.py: 这是以 dbgpt 名义发布的核心库。

  • 严格审查公共 API、构造函数参数、返回类型、Pydantic 字段、序列化格式和异常语义的向后兼容性。
  • 不得引入或扩大 core 对 dbgpt_ext、dbgpt_serve、dbgpt_app、dbgpt_client 或
    dbgpt_sandbox 的反向依赖。只报告当前 diff 新增或扩大的依赖违规问题。
  • 对于 AWEL 变更,应同时审查同步、异步和流式执行路径。检查 DAG 关系、
    ContextVar 传播、背压、顺序、结束信号、异常传播和取消处理。
  • 不得在 async 函数中直接执行阻塞式数据库、文件、网络或模型操作。检查连接、
    task、thread、generator 和临时资源的清理。
  • 公共行为变更必须提供邻近的回归测试,并覆盖相关的正常、错误、空输入、并发、
    取消或流式场景。

Files:

  • packages/dbgpt-core/src/dbgpt/util/cli/_module.py
🪛 Ruff (0.15.21)
packages/dbgpt-core/src/dbgpt/util/cli/_module.py

[warning] 23-23: Do not catch blind exception: Exception

(BLE001)

Comment on lines +15 to +24
try:
metadata = getattr(cls, "metadata", None)
if metadata is None:
return False
# For class-level properties or descriptors, get the actual metadata object
if callable(metadata):
metadata = metadata()
return issubclass(cls, BaseOperator) and isinstance(metadata, ViewMetadata)
except Exception:
return False

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add regression tests for metadata resolution.

Cover normal ViewMetadata, callable providers, missing/invalid metadata, providers that raise, unrelated classes with callable metadata, and the full _scan_awel_flow path.

As per path instructions, “公共行为变更必须提供邻近的回归测试,并覆盖相关的正常、错误、空输入、并发、取消或流式场景。”

🧰 Tools
🪛 Ruff (0.15.21)

[warning] 23-23: Do not catch blind exception: Exception

(BLE001)

Source: Path instructions

Comment on lines +16 to +21
metadata = getattr(cls, "metadata", None)
if metadata is None:
return False
# For class-level properties or descriptors, get the actual metadata object
if callable(metadata):
metadata = metadata()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Check BaseOperator before invoking metadata.

_scan_awel_flow uses base_class=object, so this predicate runs for every discovered class. Lines [20]-[21] invoke any callable metadata before confirming that cls is a BaseOperator; an unrelated class with a no-argument metadata method can therefore execute during startup. Move the subclass check ahead of metadata resolution.

Comment on lines +20 to +22
if callable(metadata):
metadata = metadata()
return issubclass(cls, BaseOperator) and isinstance(metadata, ViewMetadata)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reuse the resolved metadata downstream.

For callable cls.metadata, this helper returns True after resolving a ViewMetadata, but _scan_awel_flow later reads cls.metadata directly at Line [50], so it receives the callable and .parameters raises. It also re-evaluates the provider at Line [70]. Return or cache the resolved metadata and reuse the same object for extraction and classification.

@chen-alan

Copy link
Copy Markdown
Collaborator

Thanks for the contribution! This change is behavior-neutral for existing flow operators (no regression), but two things to confirm:

  1. The Closes #2280 linkage doesn't appear to hold. _is_flow_operator is only used by _scan_awel_flow, which is invoked solely by the gen_compat maintainers' CLI — not the dbgpt_server.py startup path where the no such table: gpts_app_collection error in [Bug] [Module Name] Bug title  #2280 actually occurs; and [Bug] [Module Name] Bug title  #2280's original cause was already addressed by making _get_app_list() lazy (agent/resource/app.py:58-116). Recommend removing Closes #2280 so the issue isn't auto-closed by mistake.
  2. The new if callable(metadata): metadata = metadata() branch looks ineffective. For @property-style metadata, accessing it on the class yields the property object (not callable), so the branch never fires; and every operator in the repo declares metadata = ViewMetadata(...) as a class attribute, so no class hits it. If descriptor-style metadata support is genuinely needed, use inspect.getattr_static or call fget on a property, and add a unit test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [Module Name] Bug title

2 participants