Skip to content

Conversation

@christopher-tin-atlan
Copy link
Collaborator

  • Add new workflows.py module with 6 workflow retrieval functions
  • Register 6 new MCP tools with comprehensive LLM-friendly docstrings
  • Add metadata extraction functions for WorkflowTemplate and Workflow types
  • Include naming convention notes and usage guidance in all tool docstrings
  • Implement error handling and type safety improvements
  • Export all workflow functions in tools/init.py

…tion

- Add new workflows.py module with 6 workflow retrieval functions
- Implement two-tier field naming convention (run_* and workflow_* prefixes)
- Register 6 new MCP tools with comprehensive LLM-friendly docstrings
- Add metadata extraction functions for WorkflowTemplate and Workflow types
- Include naming convention notes and usage guidance in all tool docstrings
- Implement error handling and type safety improvements
- Export all workflow functions in tools/__init__.py
- Remove get_workflow_run_by_id function as it provides no additional value over get_workflow_runs
- Standardize terminology: workflow = WorkflowTemplate, workflow_run = Workflow
- Update all docstrings to use consistent 'workflow' and 'workflow_run' terminology
- Clarify that only functions starting with get_workflow_run return workflow_run information
- Update LLM instructions in server.py to match implementation
@christopher-tin-atlan
Copy link
Collaborator Author

@christopher-tin-atlan
Copy link
Collaborator Author

@abhinavmathur-atlan - I've consolidated all the workflows to three tools:

  • get workflow package names; list all the packages available
  • get workflow; this can get multiple workflows by type OR a single workflow
  • get workflow runs; this can get all runs of a workflow by a single status OR get any workflow runs within list of allowed status

My validation test is here: https://atlanhq.atlassian.net/browse/CSA-1117

Let me know if you approve!

- package_name, package_version: Package information
- source_system, source_category, workflow_type: Source information
- certified, verified: Certification status
- creator_email, creator_username: Creator information
Copy link
Member

Choose a reason for hiding this comment

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

we should not send all this information

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed

  • package_version
  • source_system
  • source_category
  • workflow_type
  • certified
  • verified

I left the creator information and package name, as it does give useful information for our end users on who last created/modified the package internally which they can use for troubleshooting.

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Noted I saw your request on removing PII data in later comments. I have removed created/modified user information.

- creation_timestamp: When template was created
- package_author, package_description, package_repository: Package metadata
- workflow_steps: Workflow steps (only when getting by ID)
- workflow_spec: Full workflow specification (only when getting by ID)
Copy link
Member

Choose a reason for hiding this comment

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

do we need to send the spec? What use cases are you targetting with this tool?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. workflow_specs has been removed.

Keeping workflow_steps as it is useful to identify patterns of failures which can help the LLM make recommendations on possible issues. However, it does expose our internal pipeline patterns, which is visible in the UI, so I'm assuming that is not an issue.

image

def get_workflow_runs_tool(
workflow_name: str | None = None,
workflow_phase: str | None = None,
status: str
Copy link
Member

Choose a reason for hiding this comment

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

Looks like status and workflow_phase seem to do the same thing

f"Retrieving workflows with workflow_package_name={workflow_package_name}, max_results={max_results}"
)

logger.debug(
Copy link
Member

Choose a reason for hiding this comment

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

Remove this since we already have an info log


return {
"workflows": [processed_workflow] if processed_workflow else [],
"total": 1 if processed_workflow else 0,
Copy link
Member

Choose a reason for hiding this comment

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

this is already check above

- total: Total count of workflow_runs
- error: None if no error occurred, otherwise the error message
Examples:
Copy link
Member

Choose a reason for hiding this comment

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

Examples etc. are not required here since those are present in the tools. Same for all functions in this file


# Use workflow_phase if provided, otherwise use first item from status list
phase_to_use = workflow_phase
if not phase_to_use and status:
Copy link
Member

Choose a reason for hiding this comment

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

This whole block looks complicated and can be solved with a pydantic model

# Process the response
runs = []
total = 0
if response and response.hits and response.hits.hits:
Copy link
Member

Choose a reason for hiding this comment

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

isn't total always len(processed_runs)?

…ove API terminology. Simplified descriptions to enhance understanding of 'workflow' and 'workflow_run' distinctions.
…low and workflow_run terminology. Removed API-specific language for improved readability and consistency.
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.

4 participants