add existing sub-workflow inside workflow #201
Conversation
📝 WalkthroughWalkthroughTwo workflow service files are updated to support subworkflow inlining: extracting subworkflow references from YAML, fetching their definitions from cloud storage, and merging them into parent workflows before building instances through AriumBuilder. Changes
Sequence DiagramsequenceDiagram
participant Client
participant WorkflowService
participant YAMLParser
participant CloudStorage
participant AriumBuilder
Client->>WorkflowService: validate/create workflow with YAML
WorkflowService->>YAMLParser: extract subworkflow references
YAMLParser-->>WorkflowService: list of references (namespace/workflow_name)
alt Has Subworkflow References
loop For each reference
WorkflowService->>CloudStorage: fetch subworkflow YAML
CloudStorage-->>WorkflowService: subworkflow definition
end
rect rgb(200, 220, 240)
Note over WorkflowService,YAMLParser: Inline References Phase
WorkflowService->>YAMLParser: _inline_subworkflow_references()
YAMLParser->>YAMLParser: merge subworkflow configs
YAMLParser-->>WorkflowService: inlined YAML
end
end
WorkflowService->>AriumBuilder: build with inlined YAML
AriumBuilder-->>WorkflowService: workflow instance
WorkflowService-->>Client: validated/created workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 ?
Summary by CodeRabbit
Release Notes
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.