add existing sub-workflow inside workflow#201
Conversation
📝 WalkthroughWalkthroughTwo workflow service files now detect referenced subworkflows in ChangesSubworkflow Inlining
Sequence DiagramsequenceDiagram
participant Client
participant WorkflowInferenceService
participant CloudStorage
participant YAMLParser
participant AriumBuilder
Client->>WorkflowInferenceService: create/validate workflow (YAML)
WorkflowInferenceService->>YAMLParser: parse YAML and extract arium.ariums
YAMLParser-->>WorkflowInferenceService: arium entries
WorkflowInferenceService->>WorkflowInferenceService: detect subworkflow references
alt Has Subworkflow References
loop For each reference
WorkflowInferenceService->>CloudStorage: fetch referenced workflow YAML
CloudStorage-->>WorkflowInferenceService: referenced YAML
WorkflowInferenceService->>YAMLParser: extract referenced arium config
YAMLParser-->>WorkflowInferenceService: arium config
end
WorkflowInferenceService->>YAMLParser: inline referenced arium configs into parent YAML (preserve inherit_variables, normalize names)
YAMLParser-->>WorkflowInferenceService: inlined YAML
end
WorkflowInferenceService->>AriumBuilder: build workflow from inlined YAML
AriumBuilder-->>WorkflowInferenceService: workflow instance
WorkflowInferenceService-->>Client: return workflow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In
@wavefront/server/modules/agents_module/agents_module/services/workflow_crud_service.py:
- Line 303: The shallow copy of subworkflow configs using
subworkflow_configs[arium_name].copy() can lead to mutations of cached configs;
replace the shallow copy with a deep copy by importing and using copy.deepcopy
when creating inline_config in workflow_crud_service (the assignment to
inline_config referencing subworkflow_configs and arium_name) so nested
structures are fully cloned and cached entries aren't mutated.
In
@wavefront/server/modules/agents_module/agents_module/services/workflow_inference_service.py:
- Line 272: The current use of subworkflow_configs[arium_name].copy() creates a
shallow copy and can allow nested structures to be mutated; replace it with a
deep copy by importing the copy module and using
copy.deepcopy(subworkflow_configs[arium_name]) when assigning inline_config so
changes to inline_config (e.g., nested keys like 'agents' or 'workflow') won’t
mutate the cached subworkflow_configs entry.
♻️ Duplicate comments (1)
wavefront/server/modules/agents_module/agents_module/services/workflow_crud_service.py (1)
88-122: Duplicate ofworkflow_inference_service.pyimplementation.As noted in the other file, this method duplicates logic. The shared utility approach would benefit both services.
🧹 Nitpick comments (3)
wavefront/server/modules/agents_module/agents_module/services/workflow_inference_service.py (2)
109-143: Duplicate implementation withworkflow_crud_service.py.This method is nearly identical to
_extract_subworkflow_referencesinworkflow_crud_service.py(lines 88-122). Consider extracting this logic into a shared utility module to avoid code duplication and ensure consistent behavior across both services.Suggested approach
Create a shared utility in
agents_module/utils/workflow_utils.py:def extract_subworkflow_references(yaml_content: str) -> List[str]: """Extract subworkflow references from workflow YAML.""" # ... shared implementationThen import and use it in both services.
220-290: Add recursive processing and circular reference detection for nested subworkflowsThe current implementation only handles single-level subworkflow references. If a subworkflow contains references to other subworkflows, those nested references won't be inlined. Additionally, there's no detection for circular dependencies (e.g., A→B→A).
Consider:
- Recursively extracting and inlining subworkflow references from fetched YAML, or
- Adding a depth limit and tracking visited subworkflows to prevent circular reference issues
wavefront/server/modules/agents_module/agents_module/services/workflow_crud_service.py (1)
238-244: Chain the exception for better debugging context.When re-raising as
ValueError, preserve the original exception chain to aid debugging.Proposed fix
except CloudStorageFileNotFoundError: logger.error( f'YAML not found in cloud storage for workflow: {namespace}/{workflow_name}' ) - raise ValueError( + raise ValueError( f'Workflow YAML not found for workflow: {namespace}/{workflow_name}' - ) + ) from NoneUsing
from Noneexplicitly suppresses the chain if the original exception isn't useful, or usefrom eto preserve it:- except CloudStorageFileNotFoundError: + except CloudStorageFileNotFoundError as e: ... - raise ValueError(...) + raise ValueError(...) from e
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wavefront/server/modules/agents_module/agents_module/services/workflow_crud_service.pywavefront/server/modules/agents_module/agents_module/services/workflow_inference_service.py
🧰 Additional context used
🧬 Code graph analysis (2)
wavefront/server/modules/agents_module/agents_module/services/workflow_inference_service.py (1)
wavefront/server/modules/agents_module/agents_module/services/workflow_crud_service.py (2)
_extract_subworkflow_references(88-122)_inline_subworkflow_references(251-321)
wavefront/server/modules/agents_module/agents_module/services/workflow_crud_service.py (1)
wavefront/server/modules/agents_module/agents_module/services/workflow_inference_service.py (2)
_extract_subworkflow_references(109-143)_inline_subworkflow_references(220-290)
🪛 Ruff (0.14.10)
wavefront/server/modules/agents_module/agents_module/services/workflow_inference_service.py
140-140: Consider moving this statement to an else block
(TRY300)
141-141: Do not catch blind exception: Exception
(BLE001)
142-142: Use explicit conversion flag
Replace with conversion flag
(RUF010)
wavefront/server/modules/agents_module/agents_module/services/workflow_crud_service.py
119-119: Consider moving this statement to an else block
(TRY300)
120-120: Do not catch blind exception: Exception
(BLE001)
121-121: Use explicit conversion flag
Replace with conversion flag
(RUF010)
242-244: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
242-244: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
wavefront/server/modules/agents_module/agents_module/services/workflow_inference_service.py (1)
311-319: LGTM!The subworkflow inlining is correctly placed before agent reference extraction, ensuring that agents defined in subworkflows are also properly resolved.
wavefront/server/modules/agents_module/agents_module/services/workflow_crud_service.py (1)
343-351: LGTM!The subworkflow inlining during validation ensures that referenced subworkflows are properly resolved and validated as part of the parent workflow.
| logger.info(f'Inlining subworkflow: {arium_name}') | ||
|
|
||
| # Get the fetched subworkflow config | ||
| inline_config = subworkflow_configs[arium_name].copy() |
There was a problem hiding this comment.
Same shallow copy concern as in workflow_inference_service.py.
Use copy.deepcopy() to avoid potential mutation of cached subworkflow configurations.
Proposed fix
- inline_config = subworkflow_configs[arium_name].copy()
+ import copy
+ inline_config = copy.deepcopy(subworkflow_configs[arium_name])Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@wavefront/server/modules/agents_module/agents_module/services/workflow_crud_service.py
around line 303, The shallow copy of subworkflow configs using
subworkflow_configs[arium_name].copy() can lead to mutations of cached configs;
replace the shallow copy with a deep copy by importing and using copy.deepcopy
when creating inline_config in workflow_crud_service (the assignment to
inline_config referencing subworkflow_configs and arium_name) so nested
structures are fully cloned and cached entries aren't mutated.
| logger.info(f'Inlining subworkflow: {arium_name}') | ||
|
|
||
| # Get the fetched subworkflow config | ||
| inline_config = subworkflow_configs[arium_name].copy() |
There was a problem hiding this comment.
Shallow copy may cause unintended side effects.
Using .copy() creates a shallow copy. If subworkflow_arium contains nested dictionaries (like agents, workflow, etc.), modifications to inline_config could mutate the cached original, causing subtle bugs in subsequent workflow builds.
Proposed fix
- inline_config = subworkflow_configs[arium_name].copy()
+ import copy
+ inline_config = copy.deepcopy(subworkflow_configs[arium_name])Or import copy at the top of the file.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| inline_config = subworkflow_configs[arium_name].copy() | |
| import copy | |
| inline_config = copy.deepcopy(subworkflow_configs[arium_name]) |
🤖 Prompt for AI Agents
In
@wavefront/server/modules/agents_module/agents_module/services/workflow_inference_service.py
around line 272, The current use of subworkflow_configs[arium_name].copy()
creates a shallow copy and can allow nested structures to be mutated; replace it
with a deep copy by importing the copy module and using
copy.deepcopy(subworkflow_configs[arium_name]) when assigning inline_config so
changes to inline_config (e.g., nested keys like 'agents' or 'workflow') won’t
mutate the cached subworkflow_configs entry.
| logger.info(f'Found subworkflow reference: {arium_name}') | ||
|
|
||
| return subworkflow_references | ||
| except Exception as e: |
There was a problem hiding this comment.
Why are we catching this here ? Shouldnt we just throw exception, and let the caller know ?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wavefront/server/modules/agents_module/agents_module/controllers/agent_controller.py (1)
151-154: 💤 Low valueConsider logging a warning when
llm_inference_config_idis provided in v2 requests.The docstring correctly states that
llm_inference_config_idis ignored in v2, but the field is silently discarded if provided in the payload. This could confuse users migrating from v1 to v2 or using incorrect API versions.Consider adding a log warning to help users detect misuse:
📋 Suggested addition
After line 169, add:
# Process inputs using common utility function resolved_inputs = process_inference_inputs(agent_inference_payload.inputs) + +if agent_inference_payload.llm_inference_config_id: + logger.warning( + f"llm_inference_config_id provided in v2 request but will be ignored. " + f"LLM is resolved from agent YAML. agent_id: {agent_id}" + ) try:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wavefront/server/modules/agents_module/agents_module/controllers/agent_controller.py` around lines 151 - 154, Add a warning log when a v2 request payload includes llm_inference_config_id even though it is ignored; in agent_controller.py, inside the code path that resolves the LLM from the agent YAML (the branch where agent.model.provider == "rootflo" and v2 behavior applies), detect if payload contains llm_inference_config_id and emit a logger.warning including the agent identifier and that llm_inference_config_id is ignored in v2; do not change existing resolution logic—only add the non-breaking warning log so users are informed when they supply the ignored llm_inference_config_id.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@wavefront/server/modules/agents_module/agents_module/controllers/agent_controller.py`:
- Around line 151-154: Add a warning log when a v2 request payload includes
llm_inference_config_id even though it is ignored; in agent_controller.py,
inside the code path that resolves the LLM from the agent YAML (the branch where
agent.model.provider == "rootflo" and v2 behavior applies), detect if payload
contains llm_inference_config_id and emit a logger.warning including the agent
identifier and that llm_inference_config_id is ignored in v2; do not change
existing resolution logic—only add the non-breaking warning log so users are
informed when they supply the ignored llm_inference_config_id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b74e8456-4bc6-4351-8b09-4f016ab023e2
📒 Files selected for processing (5)
wavefront/server/apps/floware/floware/server.pywavefront/server/modules/agents_module/agents_module/agents_container.pywavefront/server/modules/agents_module/agents_module/controllers/agent_controller.pywavefront/server/modules/agents_module/agents_module/services/agent_inference_service.pywavefront/server/modules/agents_module/agents_module/services/workflow_inference_service.py
💤 Files with no reviewable changes (2)
- wavefront/server/modules/agents_module/agents_module/services/agent_inference_service.py
- wavefront/server/modules/agents_module/agents_module/services/workflow_inference_service.py
2 participants
Summary by CodeRabbit
New Features
Refactor
Chores