feat: add --separate flag for split representation of external images#809
feat: add --separate flag for split representation of external images#809patrickhoefler merged 2 commits intomainfrom
Conversation
Implements the "split representation" feature requested in #687. Any external image can now be displayed as separate nodes per usage instead of a single shared node, matching the existing --scratch separated behaviour. --separate ubuntu,alpine Each usage of a listed image gets its own graph node (ubuntu-0, ubuntu-1, …), decluttering graphs where many stages share a common base image. The --scratch flag continues to work independently. Related audit fixes included in this change: - Replace stringly-typed scratchMode string with exported ScratchMode type and constants (ScratchCollapsed, ScratchSeparated, ScratchHidden) in the dockerfile2dot package; cmd converts the validated enum value via scratchModeFromString before passing it down - BuildDotFile now accepts float64 for nodesep/ranksep instead of pre-formatted strings; formatting stays in the package that owns it - Replace O(n²) linear scan in addExternalImages with a map[string]struct{} seen-set - Remove dead layerIndex variable from dockerfileToSimplifiedDockerfile Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new CLI option to render selected external base images as separate graph nodes per usage (similar to existing scratch “separated” behavior), while also tightening the dockerfile2dot package API by introducing typed enums and moving numeric formatting into the package.
Changes:
- Add
--separateflag to split specified external images into per-usage nodes (e.g.ubuntu-0,ubuntu-1). - Replace string scratch-mode plumbing with an exported
ScratchModeenum indockerfile2dot. - Change
BuildDotFileto acceptnodesep/ranksepasfloat64and format them internally.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/dockerfile2dot/structs.go | Introduces ScratchMode enum/constants for scratch rendering behavior. |
| internal/dockerfile2dot/load.go | Updates parsing API to accept ScratchMode and separateImages. |
| internal/dockerfile2dot/load_test.go | Updates tests for new parsing signature and defaults. |
| internal/dockerfile2dot/convert.go | Implements external image separation, refactors external-image collection, updates scratch handling. |
| internal/dockerfile2dot/convert_test.go | Adds/updates tests for separation behavior and typed scratch modes. |
| internal/dockerfile2dot/build.go | Updates graph spacing params to float64 and formats attributes internally. |
| internal/dockerfile2dot/build_test.go | Updates tests to pass numeric nodesep/ranksep. |
| internal/cmd/root.go | Adds --separate, converts scratch flag string to ScratchMode, passes new args through. |
| internal/cmd/root_test.go | Adds CLI integration tests for --separate and updates help text. |
| README.md | Documents the new --separate flag and shows it in the options list. |
Comments suppressed due to low confidence (1)
internal/dockerfile2dot/convert.go:265
addExternalImagestreats anyWaitForID not present in thestagesname set as an external image. This misclassifies numeric stage references (e.g.,COPY --from=0 ...) as external images, causing an extra disconnected external image node labeled "0" to be emitted. Consider detecting in-range numeric stage indices (similar togetWaitForNodeIDin build.go) and skipping them here so they are not added asExternalImages.
for stageIndex, stage := range simplifiedDockerfile.Stages {
for layerIndex, layer := range stage.Layers {
for waitForIndex, waitFor := range layer.WaitFors {
// Skip if this references an internal stage (not an external image)
if _, ok := stages[waitFor.ID]; ok {
continue
}
imageID := waitFor.ID
originalName := waitFor.ID
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Numeric stage references (COPY --from=0) were misclassified as external images, producing a spurious disconnected node. Now detect in-range numeric indices (mirroring getWaitForNodeID) and skip them. - Trim whitespace from --separate values so inputs like --separate "ubuntu, alpine" work as expected. - Extract resolveExternalImageID helper to keep cyclomatic complexity within the linter threshold. - Explicitly set scratchMode: ScratchCollapsed in new test cases instead of relying on the zero value, so intent is clear and the tests are resilient to enum reordering. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Implements the "split representation" feature requested in #687. Any external image can now be displayed as separate nodes per usage instead of a single shared node, matching the existing --scratch separated behaviour.
--separate ubuntu,alpine
Each usage of a listed image gets its own graph node (ubuntu-0, ubuntu-1, …), decluttering graphs where many stages share a common base image. The --scratch flag continues to work independently.
Related fixes included in this change:
Closes #687