Add embedded action step expansion to --dag-ascii output#155
Conversation
Display internal steps from embedded action's referenced YAML files inline with tree formatting (├/└). The filename is shown at the end with dim color. Template variables in paths are properly expanded using the same evalVars logic as runtime execution. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR adds the ability to expand embedded action steps inline in --dag-ascii output, displaying the internal steps from referenced YAML files with tree formatting and filename annotations.
Changes:
- Embedded actions now display their internal steps with tree characters (├/└) in DAG ASCII output
- Template variable expansion is applied to embedded action paths using workflow vars
- Test coverage added for the expanded embedded action display format
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| dag_ascii.go | Implements embedded step expansion logic with path resolution, template variable evaluation, and tree-formatted rendering |
| dag_ascii_test.go | Adds embedded_expanded golden test case to verify the new functionality |
| testdata/dag_ascii/embedded_expanded.golden.txt | Golden file showing expected output format for expanded embedded actions |
| probe.go | Sets the basePath field on workflow for resolving relative paths in embedded actions |
| workflow.go | Adds basePath field and documentation comment for evalVars method |
| workflow_test.go | Renames test function to follow Go private function naming convention |
Comments suppressed due to low confidence (1)
testdata/dag_ascii/embedded_expanded.golden.txt:1
- The ANSI escape code appears to be malformed. '[2m' should be '\033[2m' or '\x1b[2m' for dim formatting, and '[0m' should be '\033[0m' or '\x1b[0m' for reset. The current output suggests the escape sequences are being corrupted or improperly rendered.
╭───────────────────────╮
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| maxFilenameWidth := innerWidth - filePrefixWidth | ||
| truncatedFilename := truncateWithEllipsis(filename, maxFilenameWidth) | ||
| // Apply dim color | ||
| dimFilename := ansiDim + truncatedFilename + ansiReset |
There was a problem hiding this comment.
ANSI escape codes should not be included when calculating the actual width. The current code truncates based on visual width but then wraps with ANSI codes, which can cause the total string length (including escape codes) to exceed the expected width. The truncation should occur before applying ANSI formatting to ensure the final rendered width matches expectations.
dag_ascii.go
Outdated
| return expanded | ||
| } | ||
|
|
||
| // resolvePath resolves a relative path, trying multiple base directories |
There was a problem hiding this comment.
This function tries multiple resolution strategies but lacks documentation explaining the priority order and fallback behavior. Consider adding a comment describing: (1) workflow directory resolution, (2) parent directory resolution for project-root relative paths, and (3) current directory fallback.
| // resolvePath resolves a relative path, trying multiple base directories | |
| // resolvePath resolves a (potentially relative) path using a fixed priority order: | |
| // 1. If the path is absolute, it is returned as-is. | |
| // 2. If workflow.basePath is set, first try the path relative to the workflow | |
| // directory (workflow.basePath/path). | |
| // 3. If not found, try the path relative to the parent of the workflow directory | |
| // (for project-root relative paths; parentDir/path). | |
| // 4. If still not found, fall back to resolving the path from the current working | |
| // directory using filepath.Abs, which matches the runtime's default behavior. |
Document the priority order and fallback behavior for path resolution. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Code Metrics Report
Details | | main (52a5483) | #155 (29b2942) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 54.8% | 55.0% | +0.2% |
| Files | 64 | 64 | 0 |
| Lines | 6439 | 6518 | +79 |
+ | Covered | 3529 | 3590 | +61 |
- | Code to Test Ratio | 1:1.0 | 1:1.0 | -0.1 |
| Code | 12684 | 12808 | +124 |
| Test | 13180 | 13180 | 0 |
- | Test Execution Time | 24s | 26s | +2s |Code coverage of files in pull request scope (83.3% → 82.4%)
Reported by octocov |
Summary
Test plan
go test -run TestDagAsciito verify all tests passembedded_expandedgolden test case🤖 Generated with Claude Code