Add --provenance flag to atmos describe component
#1584
Conversation
…tacks Add provenance tracking display that shows the source file and line number for each configuration value in component and stack descriptions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
osterman
added
the
no-release
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1584 +/- ##
==========================================
+ Coverage 62.11% 63.08% +0.97%
==========================================
Files 316 325 +9
Lines 35541 37354 +1813
==========================================
+ Hits 22077 23566 +1489
- Misses 11513 11803 +290
- Partials 1951 1985 +34
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughAdds end-to-end import provenance: CLI flag and exec branching, per-stack MergeContext provenance storage, provenance-aware YAML unmarshal/merge, provenance model & storage, inline/side-by-side/tree renderers, YAML position tracking, many tests/benchmarks, docs, and misc dependency/UI/import updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI
participant Exec as Exec/Describe
participant SP as StackProcessor
participant YAML as YAML Unmarshal
participant Merge as Merge (provenance)
participant Prov as ProvenanceStorage
participant Renderer
participant IO
User->>CLI: atmos describe component --provenance
CLI->>Exec: ExecuteDescribeComponent(params{Provenance:true,...})
Exec->>SP: process stacks (per-stack MergeContext)
SP->>YAML: Unmarshal files -> data + PositionMap
YAML-->>SP: data + positions
SP->>Merge: MergeWithProvenance(inputs, ctx, positions)
Merge->>Prov: Record provenance entries (path → ProvenanceEntry)
Merge-->>SP: mergedData
Exec->>Renderer: RenderInlineProvenance(mergedData, ctx, stackFile)
Renderer-->>Exec: renderedText
Exec->>IO: writeOutputToFile / stdout
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
cmd/describe_component.go(3 hunks)cmd/describe_stacks.go(2 hunks)internal/exec/describe_component.go(5 hunks)internal/exec/describe_stacks.go(4 hunks)pkg/ui/provenance/renderer.go(1 hunks)pkg/ui/provenance/renderer_test.go(1 hunks)website/docs/cli/commands/describe/describe-component.mdx(3 hunks)website/docs/cli/commands/describe/describe-stacks.mdx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/docs/cli/commands/describe/describe-component.mdxwebsite/docs/cli/commands/describe/describe-stacks.mdx
website/docs/cli/commands/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
website/docs/cli/commands/**/*.mdx: All new commands/flags/parameters must have Docusaurus documentation under website/docs/cli/commands//.mdx using definition lists for arguments and flags.
Follow Docusaurus conventions (front matter, Purpose note, usage, examples; consistent section ordering: Usage → Examples → Arguments → Flags).
Files:
website/docs/cli/commands/describe/describe-component.mdxwebsite/docs/cli/commands/describe/describe-stacks.mdx
website/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
website/docs/**/*.{md,mdx}: Build the website after modifying any docs under website/docs/ to verify no broken links or formatting issues.
Document user-facing template functions in the website documentation.
Files:
website/docs/cli/commands/describe/describe-component.mdxwebsite/docs/cli/commands/describe/describe-stacks.mdx
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/ui/provenance/renderer_test.gopkg/ui/provenance/renderer.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Use table-driven tests for unit tests.
Always use t.Skipf(...) with a clear reason when skipping tests; do not use t.Skip().
TestMain must call os.Exit(m.Run()) to propagate the test exit code for CLI tests.
Co-locate test files with implementation (use _test.go alongside .go files) and maintain naming symmetry with implementation files.
Files:
pkg/ui/provenance/renderer_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All Go comments must end with periods. Enforced by golangci-lint's godot.
Organize imports into three groups (stdlib, third-party, Atmos) separated by blank lines; sort alphabetically within each group; maintain existing aliases.
Add defer perf.Track(...) to all public functions and critical private functions, with a blank line after the call and package-prefixed name (e.g., "exec.ProcessComponent"). Use atmosConfig if available, otherwise nil.
Wrap all errors using static errors defined in errors/errors.go; never return dynamic errors directly.
Use the error wrapping pattern: fmt.Errorf("%w: details", errUtils.ErrStaticError, ...) with %w to preserve error chains and add context after the static error.
Bind environment variables with viper.BindEnv() and provide ATMOS_ alternatives for every env var (e.g., viper.BindEnv("ATMOS_GITHUB_TOKEN", "GITHUB_TOKEN")).
Ensure cross-platform compatibility: prefer SDKs over invoking binaries, use filepath.Join, os.PathSeparator/PathListSeparator, and runtime.GOOS when necessary.
Files:
pkg/ui/provenance/renderer_test.gointernal/exec/describe_stacks.gopkg/ui/provenance/renderer.gointernal/exec/describe_component.gocmd/describe_stacks.gocmd/describe_component.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Do not use logging for UI output. Send prompts/status/progress and actionable errors to stderr; send data/results to stdout; keep logging for system/debug with structured fields.
Most text UI must go to stderr; only data/results go to stdout. Prefer utils.PrintfMessageToTUI for UI messages.
For non-standard execution paths, capture telemetry using telemetry.CaptureCmd(...) or telemetry.CaptureCmdString(...); never capture user data.
Files:
pkg/ui/provenance/renderer_test.gointernal/exec/describe_stacks.gopkg/ui/provenance/renderer.gointernal/exec/describe_component.gocmd/describe_stacks.gocmd/describe_component.go
{internal/**/tui/**/*.go,pkg/ui/**/*.go}
📄 CodeRabbit inference engine (CLAUDE.md)
Use theme.Styles and theme.Colors with consistent Atmos theme colors from pkg/ui/theme/colors.go for TUI formatting.
Files:
pkg/ui/provenance/renderer_test.gopkg/ui/provenance/renderer.go
{internal/exec,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
All new configs must support Go templating using FuncMap() from internal/exec/template_funcs.go.
Files:
pkg/ui/provenance/renderer_test.gointernal/exec/describe_stacks.gopkg/ui/provenance/renderer.gointernal/exec/describe_component.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/exec/describe_stacks.gopkg/ui/provenance/renderer.gointernal/exec/describe_component.gocmd/describe_stacks.gocmd/describe_component.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriate
cmd/**/*.go: Render markdown examples using utils.PrintfMarkdown() in Cobra commands that embed example usage.
One Cobra command per file in cmd/.
Files:
cmd/describe_stacks.gocmd/describe_component.go
🧠 Learnings (3)
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
internal/exec/describe_stacks.gointernal/exec/describe_component.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
PR: cloudposse/atmos#1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-01-19T15:49:15.593Z
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Applied to files:
website/docs/cli/commands/describe/describe-stacks.mdx
🧬 Code graph analysis (4)
pkg/ui/provenance/renderer_test.go (2)
pkg/schema/schema.go (7)
AtmosConfiguration(27-63)ConfigSourcesStackDependencies(693-693)ConfigSources(701-701)ConfigSourcesItem(695-699)Settings(679-683)AtmosSettings(249-269)Terminal(206-214)pkg/ui/provenance/renderer.go (1)
NewProvenanceRenderer(27-33)
internal/exec/describe_stacks.go (6)
pkg/schema/schema.go (2)
AtmosConfiguration(27-63)ConfigSources(701-701)pkg/perf/perf.go (1)
Track(58-86)pkg/ui/provenance/renderer.go (1)
NewProvenanceRenderer(27-33)internal/exec/terraform_generate_planfile.go (1)
ErrInvalidFormat(21-21)pkg/utils/yaml_utils.go (1)
WriteToFileAsYAML(124-137)pkg/utils/log_utils.go (1)
PrintMessage(19-21)
pkg/ui/provenance/renderer.go (6)
pkg/schema/schema.go (6)
AtmosConfiguration(27-63)ConfigSources(701-701)Settings(679-683)Terminal(206-214)ConfigSourcesStackDependencies(693-693)ConfigSourcesItem(695-699)pkg/perf/perf.go (1)
Track(58-86)internal/tui/templates/term/term_writer.go (1)
IsTTYSupportForStdout(83-87)pkg/utils/yaml_utils.go (2)
GetHighlightedYAML(91-103)ConvertToYAML(165-181)pkg/ui/theme/colors.go (1)
ColorBorder(21-21)pkg/utils/json_utils.go (1)
GetHighlightedJSON(28-46)
internal/exec/describe_component.go (6)
pkg/schema/schema.go (2)
AtmosConfiguration(27-63)ConfigSources(701-701)pkg/perf/perf.go (1)
Track(58-86)pkg/ui/provenance/renderer.go (2)
NewProvenanceRenderer(27-33)ProvenanceRenderer(22-24)internal/exec/describe_config.go (1)
DescribeConfigFormatError(16-18)pkg/utils/yaml_utils.go (1)
WriteToFileAsYAML(124-137)pkg/utils/log_utils.go (1)
PrintMessage(19-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: website-deploy-preview
- GitHub Check: Run pre-commit hooks
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (25)
website/docs/cli/commands/describe/describe-component.mdx (3)
62-62: LGTM - Example follows established pattern.The new example demonstrates the flag usage clearly and aligns with the existing command examples.
99-100: LGTM - Flag documentation is clear.The flag description effectively communicates the purpose and behavior of
--show-provenance.
880-956: LGTM - Comprehensive provenance documentation.The documentation thoroughly covers:
- Terminal vs non-terminal output behavior
- JSON format with embedded provenance
- Practical use cases
- Connection to existing
sourcessectionThe examples and explanations provide clear guidance for users.
website/docs/cli/commands/describe/describe-stacks.mdx (2)
53-54: LGTM - Examples are consistent.The new examples demonstrate the flag usage clearly and follow the established pattern for the stacks command.
94-95: LGTM - Flag documentation is consistent.The flag description aligns with the component documentation and clearly explains the feature for the stacks context.
cmd/describe_component.go (2)
64-80: LGTM - Standard flag handling.The flag retrieval and parameter passing follow the established Cobra pattern and are consistent with the other flags in this command.
95-95: LGTM - Flag registration is correct.The flag registration follows the established pattern with an appropriate default value and clear description.
pkg/ui/provenance/renderer.go (6)
1-19: LGTM - Package structure is clean.The package declaration, imports, and constants are well-organized. The default terminal width of 120 is a reasonable fallback.
21-33: LGTM - Constructor is clean.The
ProvenanceRendererstruct and constructor follow good practices with minimal state and proper performance tracking.
35-47: LGTM - Smart TTY-aware routing.The
RenderYAMLWithProvenancemethod appropriately routes to two-column or inline rendering based on TTY support, providing a good user experience in different contexts.
49-111: LGTM - Two-column rendering is well-implemented.The two-column layout correctly:
- Equalizes line counts with padding
- Calculates column widths based on terminal configuration
- Uses theme styles for consistent appearance
- Applies proper separators
The implementation provides a clean visual experience for TTY output.
113-144: LGTM - Inline comment rendering is appropriate for non-TTY.The inline comment approach works well for piped output, appending provenance information as YAML comments. The key matching via
extractKeyFromLineis intentionally simple and works for the common cases.
146-278: LGTM - Helper functions are well-structured.The helper functions are:
- Focused and single-purpose
- Properly instrumented with performance tracking
- Handle edge cases (empty deps, non-map data)
- Use appropriate data structures
The
extractKeyFromLineimplementation is intentionally simple, extracting keys up to the first colon, which works for the common YAML structure cases.internal/exec/describe_component.go (4)
12-12: LGTM - Import is correct.The provenance package import follows the established import organization pattern.
25-25: LGTM - Field addition is clean.The
ShowProvenancefield is appropriately added to theDescribeComponentParamsstruct.
96-103: LGTM - Clean opt-in flow.The early return when
showProvenanceis true cleanly separates the provenance rendering path from the existing pager flow, preventing double output.
153-169: LGTM - Provenance rendering is well-structured.The
renderWithProvenancefunction:
- Gracefully falls back to regular rendering when no sources exist
- Delegates format-specific rendering to a helper
- Separates output writing concerns
- Includes proper performance tracking
pkg/ui/provenance/renderer_test.go (1)
1-627: LGTM - Comprehensive test coverage.The test suite provides excellent coverage with 18 test cases covering:
- Public API (constructor, render methods)
- Internal helpers (formatting, building, embedding)
- Edge cases (empty deps, non-matching sources, non-map data)
- Different rendering modes (TTY vs non-TTY, two-column vs inline)
- Width calculations and line padding
The table-driven tests for
formatStackDependenciesandextractKeyFromLinefollow best practices.cmd/describe_stacks.go (2)
93-93: LGTM - Flag binding is correct.The flag is properly mapped to the
ShowProvenancefield in theDescribeStacksArgsstruct.
155-155: LGTM - Flag registration is consistent.The flag registration follows the established pattern with appropriate default and description, consistent with the component command.
internal/exec/describe_stacks.go (5)
16-16: LGTM - Import is correct.The provenance package import follows the established pattern.
33-33: LGTM - Field addition is clean.The
ShowProvenancefield is appropriately added toDescribeStacksArgs, matching the component implementation.
99-102: LGTM - Clean provenance opt-in.The early return when
ShowProvenanceis true cleanly separates the provenance path from the normal scroll view, consistent with the component implementation.
116-162: Rendering implementation is well-structured, but verify WriteToFileAsYAML usage.The
renderStacksWithProvenancefunction follows the same solid pattern as the component implementation:
- Graceful fallback when no sources exist
- Format-specific rendering via switch
- Proper error handling
However, verify that
u.WriteToFileAsYAMLon line 153 correctly handles theoutputstring parameter from the renderer.This is the same verification concern as in describe_component.go - the script above will check both usages.
164-234: Source extraction is clean but returns first found sources.The extraction helpers correctly navigate the nested stack structure (stack → components → componentType → component → sources) with proper type checking.
The implementation returns the first found
ConfigSources, which is appropriate for display purposes since provenance structure is consistent across components. If multiple components have different provenance that needs merging, this would need adjustment, but for the current use case of showing "a provenance view," this is sensible.
## what - Reverted incomplete provenance implementation from pkg/ui/provenance/ - Removed --show-provenance flags from describe component and describe stacks commands - Created comprehensive PRD for proper line-level provenance system at docs/prd/line-level-provenance.md ## why The initial provenance implementation had fundamental limitations: - Only tracked top-level section keys (vars.*, settings.*), not nested values - Two-column rendering was broken and couldn't align with complex YAML structures - No line number tracking, only file paths - Not reusable across different Atmos config types (stacks, atmos.yaml, vendor, workflows) - Output polluted with 750+ lines of raw sources data - Tests didn't catch critical bugs ## references - PRD: docs/prd/line-level-provenance.md - Extends existing MergeContext system (pkg/merge/merge_context.go) - Uses gopkg.in/yaml.v3 Node API for line tracking - Estimated 3-4 weeks for proper implementation - Backward compatible deprecation of current sources system 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
github-actions
bot
added
size/l
Renamed PRD to better reflect that we're tracking import/inheritance chains rather than just line-level granularity. 'Import provenance' more accurately describes tracking where values come from through the import system. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/prd/import-provenance.md (1)
100-130: Consider adding language identifier to fenced code block.The ASCII diagram is in a fenced code block without a language specifier. While ASCII art doesn't strictly need one, adding
textorasciisilences the linter.Apply this diff:
-``` +```text ┌─────────────────────────────────────────────────────────────┐ │ YAML Parser (gopkg.in/yaml.v3 Node API) │
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docs/prd/import-provenance.md(1 hunks)internal/exec/describe_component.go(1 hunks)internal/exec/describe_stacks.go(0 hunks)
💤 Files with no reviewable changes (1)
- internal/exec/describe_stacks.go
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All Go comments must end with periods. Enforced by golangci-lint's godot.
Organize imports into three groups (stdlib, third-party, Atmos) separated by blank lines; sort alphabetically within each group; maintain existing aliases.
Add defer perf.Track(...) to all public functions and critical private functions, with a blank line after the call and package-prefixed name (e.g., "exec.ProcessComponent"). Use atmosConfig if available, otherwise nil.
Wrap all errors using static errors defined in errors/errors.go; never return dynamic errors directly.
Use the error wrapping pattern: fmt.Errorf("%w: details", errUtils.ErrStaticError, ...) with %w to preserve error chains and add context after the static error.
Bind environment variables with viper.BindEnv() and provide ATMOS_ alternatives for every env var (e.g., viper.BindEnv("ATMOS_GITHUB_TOKEN", "GITHUB_TOKEN")).
Ensure cross-platform compatibility: prefer SDKs over invoking binaries, use filepath.Join, os.PathSeparator/PathListSeparator, and runtime.GOOS when necessary.
Files:
internal/exec/describe_component.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/exec/describe_component.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Do not use logging for UI output. Send prompts/status/progress and actionable errors to stderr; send data/results to stdout; keep logging for system/debug with structured fields.
Most text UI must go to stderr; only data/results go to stdout. Prefer utils.PrintfMessageToTUI for UI messages.
For non-standard execution paths, capture telemetry using telemetry.CaptureCmd(...) or telemetry.CaptureCmdString(...); never capture user data.
Files:
internal/exec/describe_component.go
{internal/exec,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
All new configs must support Go templating using FuncMap() from internal/exec/template_funcs.go.
Files:
internal/exec/describe_component.go
🧠 Learnings (2)
📚 Learning: 2025-10-02T20:13:55.539Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T20:13:55.539Z
Learning: Applies to **/*.go : Add defer perf.Track(...) to all public functions and critical private functions, with a blank line after the call and package-prefixed name (e.g., "exec.ProcessComponent"). Use atmosConfig if available, otherwise nil.
Applied to files:
internal/exec/describe_component.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
internal/exec/describe_component.go
🪛 markdownlint-cli2 (0.18.1)
docs/prd/import-provenance.md
100-100: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
- Implement MergeWithProvenance function to track provenance during YAML merging - Add provenance storage and entry management with thread-safe operations - Create tree renderer for inline provenance visualization with depth indicators - Add YAML position extraction utilities for accurate line/column tracking - Fix nested import provenance tracking in stack processor - Record provenance metadata using merge context's current file - Handle []any type for imports array in describe component output - Add comprehensive test suite for provenance tracking functionality - Support JSONPath addressing for nested configuration values - Add benchmarks for provenance operations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
github-actions
bot
added
size/xl
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cmd/describe_component.go (1)
64-96: Use the documented--show-provenanceflag name.
Docs, tests, and the PR synopsis all promisedescribe component --show-provenance, but the code registers--provenance. Users following the spec will hit “unknown flag” and lose the new feature. Please align the flag key (definition and lookup) with the published name.- provenance, err := flags.GetBool("provenance") + provenance, err := flags.GetBool("show-provenance") @@ - describeComponentCmd.PersistentFlags().Bool("provenance", false, "Enable provenance tracking to show where configuration values originated") + describeComponentCmd.PersistentFlags().Bool("show-provenance", false, "Enable provenance tracking to show where configuration values originated")internal/exec/stack_processor_utils.go (1)
101-198: Data race on errorResult.errorResult is written by multiple goroutines without synchronization; writes can race and lose earlier errors.
Apply locking to first-write and reuse it for all error assignments:
@@ - var errorResult error + var errorResult error + var errorMu sync.Mutex @@ - if err != nil { - errorResult = err - return - } + if err != nil { + errorMu.Lock() + if errorResult == nil { + errorResult = err + } + errorMu.Unlock() + return + } @@ - if err != nil { - errorResult = err - return - } + if err != nil { + errorMu.Lock() + if errorResult == nil { + errorResult = err + } + errorMu.Unlock() + return + } @@ - if err != nil { - errorResult = err - return - } + if err != nil { + errorMu.Lock() + if errorResult == nil { + errorResult = err + } + errorMu.Unlock() + return + }Repeat similarly for other goroutine error assignments in this function.
🧹 Nitpick comments (6)
pkg/provenance/inline_test.go (1)
26-50: Test doesn't verify provenance comments are added.TestRenderInline_WithProvenance only asserts that YAML content is present, but doesn't verify that provenance comments are actually appended. Since RenderInline has placeholder logic (as noted in inline.go review), this test passes despite the incomplete implementation.
Add an assertion to verify provenance comments appear in the output:
result, err := RenderInline(data, ctx) require.NoError(t, err) - // Should contain valid YAML. assert.Contains(t, result, "name: test") assert.Contains(t, result, "value: 42") + // Should contain provenance comment. + assert.Contains(t, result, "# from: config.yaml:10:5")This will fail until the RenderInline implementation is completed.
pkg/merge/merge_with_provenance_test.go (1)
4-11: Optional: run tests in parallel.Consider t.Parallel() in each test to speed up the suite; tests don’t share state.
pkg/provenance/import_provenance_test.go (2)
83-91: Stabilize output by disabling colors/highlighting in tests.RenderInlineProvenanceWithStackFile may emit ANSI. Set atmosConfig.Settings.Terminal.Color = false and disable syntax highlighting to avoid flaky Contains checks.
As per coding guidelines
Also applies to: 151-159, 244-252
166-177: Simplify symbol counting.Use strings.Count(output, "● [0]") instead of a manual search loop.
internal/exec/stack_processor_utils.go (1)
206-211: Avoid storing a shared context after wg (stale/incomplete).With per-goroutine contexts, this SetLastMergeContext(sharedMergeContext) is misleading. Remove it or aggregate provenance then set once.
- if sharedMergeContext != nil && sharedMergeContext.IsProvenanceEnabled() { - SetLastMergeContext(sharedMergeContext) - } + // Intentionally left empty. Use an aggregated context if needed.As per coding guidelines
pkg/utils/jsonpath.go (1)
9-28: Optional: avoid fmt in hot paths.AppendJSONPathIndex can use strings.Builder or strconv.AppendInt to avoid fmt overhead if on hot paths.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (28)
cmd/describe_component.go(3 hunks)internal/exec/describe_component.go(8 hunks)internal/exec/describe_component_provenance_test.go(1 hunks)internal/exec/describe_component_test.go(2 hunks)internal/exec/stack_processor_utils.go(11 hunks)pkg/merge/integration_test.go(1 hunks)pkg/merge/merge.go(1 hunks)pkg/merge/merge_context.go(5 hunks)pkg/merge/merge_context_test.go(1 hunks)pkg/merge/merge_with_provenance.go(1 hunks)pkg/merge/merge_with_provenance_test.go(1 hunks)pkg/merge/provenance_benchmark_test.go(1 hunks)pkg/merge/provenance_entry.go(1 hunks)pkg/merge/provenance_entry_test.go(1 hunks)pkg/merge/provenance_storage.go(1 hunks)pkg/merge/provenance_storage_test.go(1 hunks)pkg/provenance/import_provenance_test.go(1 hunks)pkg/provenance/inline.go(1 hunks)pkg/provenance/inline_test.go(1 hunks)pkg/provenance/tree_renderer.go(1 hunks)pkg/provenance/tree_renderer_test.go(1 hunks)pkg/schema/schema.go(1 hunks)pkg/ui/theme/colors.go(1 hunks)pkg/utils/jsonpath.go(1 hunks)pkg/utils/jsonpath_test.go(1 hunks)pkg/utils/yaml_position.go(1 hunks)pkg/utils/yaml_position_test.go(1 hunks)pkg/utils/yaml_utils.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/ui/theme/colors.gopkg/merge/merge.gopkg/utils/yaml_utils.gopkg/merge/provenance_storage_test.gopkg/provenance/tree_renderer_test.gopkg/merge/merge_with_provenance.gopkg/provenance/inline_test.gopkg/merge/integration_test.gopkg/provenance/import_provenance_test.gopkg/merge/merge_context_test.gopkg/merge/provenance_entry.gopkg/merge/provenance_benchmark_test.gopkg/merge/merge_context.gopkg/utils/jsonpath_test.gopkg/merge/provenance_storage.gopkg/schema/schema.gopkg/merge/merge_with_provenance_test.gopkg/merge/provenance_entry_test.gopkg/provenance/inline.gopkg/utils/jsonpath.gopkg/utils/yaml_position.gopkg/utils/yaml_position_test.gopkg/provenance/tree_renderer.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All Go comments must end with periods. Enforced by golangci-lint's godot.
Organize imports into three groups (stdlib, third-party, Atmos) separated by blank lines; sort alphabetically within each group; maintain existing aliases.
Add defer perf.Track(...) to all public functions and critical private functions, with a blank line after the call and package-prefixed name (e.g., "exec.ProcessComponent"). Use atmosConfig if available, otherwise nil.
Wrap all errors using static errors defined in errors/errors.go; never return dynamic errors directly.
Use the error wrapping pattern: fmt.Errorf("%w: details", errUtils.ErrStaticError, ...) with %w to preserve error chains and add context after the static error.
Bind environment variables with viper.BindEnv() and provide ATMOS_ alternatives for every env var (e.g., viper.BindEnv("ATMOS_GITHUB_TOKEN", "GITHUB_TOKEN")).
Ensure cross-platform compatibility: prefer SDKs over invoking binaries, use filepath.Join, os.PathSeparator/PathListSeparator, and runtime.GOOS when necessary.
Files:
pkg/ui/theme/colors.gopkg/merge/merge.gopkg/utils/yaml_utils.gopkg/merge/provenance_storage_test.gopkg/provenance/tree_renderer_test.gopkg/merge/merge_with_provenance.gopkg/provenance/inline_test.gopkg/merge/integration_test.gocmd/describe_component.gopkg/provenance/import_provenance_test.gopkg/merge/merge_context_test.gopkg/merge/provenance_entry.gopkg/merge/provenance_benchmark_test.gopkg/merge/merge_context.gopkg/utils/jsonpath_test.gopkg/merge/provenance_storage.gopkg/schema/schema.gopkg/merge/merge_with_provenance_test.gointernal/exec/stack_processor_utils.gointernal/exec/describe_component_provenance_test.gopkg/merge/provenance_entry_test.gointernal/exec/describe_component_test.gopkg/provenance/inline.gopkg/utils/jsonpath.gopkg/utils/yaml_position.gopkg/utils/yaml_position_test.gopkg/provenance/tree_renderer.gointernal/exec/describe_component.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/ui/theme/colors.gopkg/merge/merge.gopkg/utils/yaml_utils.gopkg/merge/merge_with_provenance.gocmd/describe_component.gopkg/merge/provenance_entry.gopkg/merge/merge_context.gopkg/merge/provenance_storage.gopkg/schema/schema.gointernal/exec/stack_processor_utils.gopkg/provenance/inline.gopkg/utils/jsonpath.gopkg/utils/yaml_position.gopkg/provenance/tree_renderer.gointernal/exec/describe_component.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Do not use logging for UI output. Send prompts/status/progress and actionable errors to stderr; send data/results to stdout; keep logging for system/debug with structured fields.
Most text UI must go to stderr; only data/results go to stdout. Prefer utils.PrintfMessageToTUI for UI messages.
For non-standard execution paths, capture telemetry using telemetry.CaptureCmd(...) or telemetry.CaptureCmdString(...); never capture user data.
Files:
pkg/ui/theme/colors.gopkg/merge/merge.gopkg/utils/yaml_utils.gopkg/merge/provenance_storage_test.gopkg/provenance/tree_renderer_test.gopkg/merge/merge_with_provenance.gopkg/provenance/inline_test.gopkg/merge/integration_test.gocmd/describe_component.gopkg/provenance/import_provenance_test.gopkg/merge/merge_context_test.gopkg/merge/provenance_entry.gopkg/merge/provenance_benchmark_test.gopkg/merge/merge_context.gopkg/utils/jsonpath_test.gopkg/merge/provenance_storage.gopkg/schema/schema.gopkg/merge/merge_with_provenance_test.gointernal/exec/stack_processor_utils.gointernal/exec/describe_component_provenance_test.gopkg/merge/provenance_entry_test.gointernal/exec/describe_component_test.gopkg/provenance/inline.gopkg/utils/jsonpath.gopkg/utils/yaml_position.gopkg/utils/yaml_position_test.gopkg/provenance/tree_renderer.gointernal/exec/describe_component.go
{internal/**/tui/**/*.go,pkg/ui/**/*.go}
📄 CodeRabbit inference engine (CLAUDE.md)
Use theme.Styles and theme.Colors with consistent Atmos theme colors from pkg/ui/theme/colors.go for TUI formatting.
Files:
pkg/ui/theme/colors.go
{internal/exec,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
All new configs must support Go templating using FuncMap() from internal/exec/template_funcs.go.
Files:
pkg/ui/theme/colors.gopkg/merge/merge.gopkg/utils/yaml_utils.gopkg/merge/provenance_storage_test.gopkg/provenance/tree_renderer_test.gopkg/merge/merge_with_provenance.gopkg/provenance/inline_test.gopkg/merge/integration_test.gopkg/provenance/import_provenance_test.gopkg/merge/merge_context_test.gopkg/merge/provenance_entry.gopkg/merge/provenance_benchmark_test.gopkg/merge/merge_context.gopkg/utils/jsonpath_test.gopkg/merge/provenance_storage.gopkg/schema/schema.gopkg/merge/merge_with_provenance_test.gointernal/exec/stack_processor_utils.gointernal/exec/describe_component_provenance_test.gopkg/merge/provenance_entry_test.gointernal/exec/describe_component_test.gopkg/provenance/inline.gopkg/utils/jsonpath.gopkg/utils/yaml_position.gopkg/utils/yaml_position_test.gopkg/provenance/tree_renderer.gointernal/exec/describe_component.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Use table-driven tests for unit tests.
Always use t.Skipf(...) with a clear reason when skipping tests; do not use t.Skip().
TestMain must call os.Exit(m.Run()) to propagate the test exit code for CLI tests.
Co-locate test files with implementation (use _test.go alongside .go files) and maintain naming symmetry with implementation files.
Files:
pkg/merge/provenance_storage_test.gopkg/provenance/tree_renderer_test.gopkg/provenance/inline_test.gopkg/merge/integration_test.gopkg/provenance/import_provenance_test.gopkg/merge/merge_context_test.gopkg/merge/provenance_benchmark_test.gopkg/utils/jsonpath_test.gopkg/merge/merge_with_provenance_test.gointernal/exec/describe_component_provenance_test.gopkg/merge/provenance_entry_test.gointernal/exec/describe_component_test.gopkg/utils/yaml_position_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriate
cmd/**/*.go: Render markdown examples using utils.PrintfMarkdown() in Cobra commands that embed example usage.
One Cobra command per file in cmd/.
Files:
cmd/describe_component.go
internal/exec/stack_processor_utils.go
📄 CodeRabbit inference engine (CLAUDE.md)
For stack processing utilities, change internal/exec/stack_processor_utils.go and ensure template rendering still works.
Files:
internal/exec/stack_processor_utils.go
internal/exec/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Add comprehensive tests for new template functions in internal/exec/.
Files:
internal/exec/describe_component_provenance_test.gointernal/exec/describe_component_test.go
🧠 Learnings (5)
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
pkg/utils/jsonpath_test.go
📚 Learning: 2025-10-02T20:13:55.579Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T20:13:55.579Z
Learning: Applies to **/*_test.go : Use table-driven tests for unit tests.
Applied to files:
pkg/utils/jsonpath_test.go
📚 Learning: 2025-10-02T20:13:55.579Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T20:13:55.579Z
Learning: Applies to internal/exec/**/*_test.go : Add comprehensive tests for new template functions in internal/exec/.
Applied to files:
internal/exec/describe_component_provenance_test.gointernal/exec/describe_component_test.go
📚 Learning: 2025-10-02T20:13:55.579Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-02T20:13:55.579Z
Learning: Applies to **/*.go : Add defer perf.Track(...) to all public functions and critical private functions, with a blank line after the call and package-prefixed name (e.g., "exec.ProcessComponent"). Use atmosConfig if available, otherwise nil.
Applied to files:
internal/exec/describe_component.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
internal/exec/describe_component.go
🧬 Code graph analysis (24)
pkg/merge/merge.go (1)
pkg/merge/merge_with_provenance.go (1)
MergeWithProvenance(19-43)
pkg/utils/yaml_utils.go (3)
pkg/schema/schema.go (1)
AtmosConfiguration(27-64)pkg/utils/yaml_position.go (2)
PositionMap(14-14)ExtractYAMLPositions(19-27)pkg/perf/perf.go (1)
Track(58-86)
pkg/merge/provenance_storage_test.go (2)
pkg/merge/provenance_storage.go (2)
NewProvenanceStorage(22-26)ProvenanceStorage(11-19)pkg/merge/provenance_entry.go (4)
ProvenanceEntry(30-49)ProvenanceTypeInline(16-16)ProvenanceTypeImport(13-13)ProvenanceTypeOverride(19-19)
pkg/provenance/tree_renderer_test.go (1)
pkg/provenance/tree_renderer.go (1)
YAMLLineInfo(42-46)
pkg/merge/merge_with_provenance.go (6)
pkg/schema/schema.go (1)
AtmosConfiguration(27-64)pkg/merge/merge_context.go (1)
MergeContext(13-33)pkg/utils/yaml_position.go (2)
PositionMap(14-14)GetYAMLPosition(105-116)pkg/perf/perf.go (1)
Track(58-86)pkg/merge/provenance_entry.go (4)
ProvenanceTypeInline(16-16)ProvenanceType(9-9)ProvenanceTypeImport(13-13)ProvenanceEntry(30-49)pkg/utils/jsonpath.go (2)
AppendJSONPathKey(35-43)AppendJSONPathIndex(50-56)
pkg/provenance/inline_test.go (3)
pkg/provenance/inline.go (2)
RenderInline(17-56)FormatProvenanceComment(59-64)pkg/merge/merge_context.go (1)
NewMergeContext(36-40)pkg/merge/provenance_entry.go (2)
ProvenanceEntry(30-49)ProvenanceTypeInline(16-16)
pkg/merge/integration_test.go (5)
pkg/utils/yaml_utils.go (1)
UnmarshalYAML(234-236)pkg/utils/yaml_position.go (1)
PositionMap(14-14)pkg/merge/merge_context.go (1)
NewMergeContext(36-40)pkg/schema/schema.go (1)
AtmosConfiguration(27-64)pkg/merge/merge_with_provenance.go (1)
MergeWithProvenance(19-43)
pkg/provenance/import_provenance_test.go (4)
pkg/merge/merge_context.go (1)
NewMergeContext(36-40)pkg/merge/provenance_entry.go (3)
ProvenanceEntry(30-49)ProvenanceTypeInline(16-16)ProvenanceTypeImport(13-13)pkg/schema/schema.go (4)
AtmosConfiguration(27-64)Settings(680-684)AtmosSettings(250-270)Terminal(207-215)pkg/provenance/tree_renderer.go (1)
RenderInlineProvenanceWithStackFile(612-711)
pkg/merge/merge_context_test.go (2)
pkg/merge/merge_context.go (2)
MergeContext(13-33)NewMergeContext(36-40)pkg/merge/provenance_entry.go (5)
ProvenanceEntry(30-49)ProvenanceTypeInline(16-16)ProvenanceTypeImport(13-13)ProvenanceTypeOverride(19-19)ProvenanceType(9-9)
pkg/merge/provenance_benchmark_test.go (3)
pkg/merge/merge_context.go (1)
NewMergeContext(36-40)pkg/merge/provenance_entry.go (2)
ProvenanceEntry(30-49)ProvenanceTypeInline(16-16)pkg/merge/provenance_storage.go (1)
NewProvenanceStorage(22-26)
pkg/merge/merge_context.go (3)
pkg/merge/provenance_storage.go (2)
ProvenanceStorage(11-19)NewProvenanceStorage(22-26)pkg/utils/yaml_position.go (1)
PositionMap(14-14)pkg/merge/provenance_entry.go (4)
ProvenanceEntry(30-49)ProvenanceType(9-9)ProvenanceTypeInline(16-16)ProvenanceTypeImport(13-13)
pkg/utils/jsonpath_test.go (1)
pkg/utils/jsonpath.go (8)
BuildJSONPath(14-28)AppendJSONPathKey(35-43)AppendJSONPathIndex(50-56)SplitJSONPath(64-106)ParseJSONPathIndex(114-126)IsJSONPathIndex(133-136)GetJSONPathParent(144-165)GetJSONPathLeaf(173-184)
pkg/merge/provenance_storage.go (1)
pkg/merge/provenance_entry.go (1)
ProvenanceEntry(30-49)
pkg/schema/schema.go (2)
pkg/store/registry.go (1)
StoreRegistry(7-7)pkg/profiler/profiler.go (1)
Config(60-66)
pkg/merge/merge_with_provenance_test.go (5)
pkg/schema/schema.go (1)
AtmosConfiguration(27-64)pkg/merge/merge_context.go (2)
NewMergeContext(36-40)MergeContext(13-33)pkg/merge/merge_with_provenance.go (1)
MergeWithProvenance(19-43)pkg/utils/yaml_position.go (1)
PositionMap(14-14)pkg/merge/provenance_entry.go (2)
ProvenanceTypeInline(16-16)ProvenanceTypeImport(13-13)
internal/exec/stack_processor_utils.go (4)
pkg/merge/merge_context.go (2)
MergeContext(13-33)NewMergeContext(36-40)pkg/utils/yaml_utils.go (1)
UnmarshalYAMLFromFileWithPositions(269-302)pkg/schema/schema.go (1)
AtmosSectionMapType(14-14)pkg/merge/provenance_entry.go (1)
ProvenanceEntry(30-49)
internal/exec/describe_component_provenance_test.go (4)
pkg/schema/schema.go (1)
ConfigAndStacksInfo(458-535)internal/exec/stack_processor_utils.go (1)
ClearLastMergeContext(57-61)internal/exec/describe_component.go (1)
ExecuteDescribeComponentWithContext(205-351)pkg/merge/merge_context.go (1)
MergeContext(13-33)
pkg/merge/provenance_entry_test.go (1)
pkg/merge/provenance_entry.go (8)
ProvenanceType(9-9)ProvenanceEntry(30-49)ProvenanceTypeInline(16-16)ProvenanceTypeImport(13-13)ProvenanceTypeOverride(19-19)ProvenanceTypeComputed(22-22)NewProvenanceEntry(52-61)ProvenanceTypeDefault(25-25)
internal/exec/describe_component_test.go (5)
pkg/logger/log.go (3)
SetLevel(84-86)SetOutput(94-96)Logf(79-81)internal/exec/describe_component.go (5)
ExecuteDescribeComponent(190-202)NewDescribeComponentExec(41-50)DescribeComponentParams(20-30)DescribeComponentResult(183-187)FilterComputedFields(355-380)internal/exec/stack_processor_utils.go (1)
GetLastMergeContext(50-54)pkg/merge/merge_context.go (1)
MergeContext(13-33)pkg/utils/yaml_utils.go (1)
ConvertToYAML(165-181)
pkg/provenance/inline.go (2)
pkg/merge/merge_context.go (1)
MergeContext(13-33)pkg/merge/provenance_entry.go (1)
ProvenanceEntry(30-49)
pkg/utils/yaml_position.go (1)
pkg/utils/jsonpath.go (2)
AppendJSONPathKey(35-43)AppendJSONPathIndex(50-56)
pkg/utils/yaml_position_test.go (1)
pkg/utils/yaml_position.go (5)
ExtractYAMLPositions(19-27)HasYAMLPosition(119-126)PositionMap(14-14)Position(8-11)GetYAMLPosition(105-116)
pkg/provenance/tree_renderer.go (6)
pkg/merge/merge_context.go (1)
MergeContext(13-33)pkg/perf/perf.go (1)
Track(58-86)pkg/merge/provenance_entry.go (6)
ProvenanceTypeComputed(22-22)ProvenanceEntry(30-49)ProvenanceTypeInline(16-16)ProvenanceTypeOverride(19-19)ProvenanceTypeImport(13-13)ProvenanceTypeDefault(25-25)pkg/ui/theme/colors.go (7)
ColorSelectedItem(22-22)ColorCyan(12-12)ColorDarkGray(15-15)ColorGreen(11-11)ColorYellow(17-17)ColorOrange(18-18)ColorRed(16-16)pkg/utils/yaml_utils.go (1)
ConvertToYAML(165-181)pkg/utils/highlight_utils.go (1)
HighlightCodeWithConfig(81-122)
internal/exec/describe_component.go (10)
pkg/merge/merge_context.go (1)
MergeContext(13-33)pkg/provenance/tree_renderer.go (1)
RenderInlineProvenanceWithStackFile(612-711)pkg/utils/log_utils.go (1)
PrintfMessageToTUI(29-31)pkg/schema/schema.go (2)
AtmosConfiguration(27-64)ConfigAndStacksInfo(458-535)pkg/perf/perf.go (1)
Track(58-86)pkg/config/config.go (1)
InitCliConfig(25-62)internal/exec/stack_processor_utils.go (2)
ClearLastMergeContext(57-61)GetLastMergeContext(50-54)internal/exec/utils.go (1)
ProcessStacks(185-573)pkg/merge/provenance_entry.go (1)
ProvenanceEntry(30-49)internal/exec/filter_utils.go (2)
GetIncludeEmptySetting(60-67)FilterEmptySections(51-58)
🪛 GitHub Check: Build (macos-latest, macos)
pkg/utils/yaml_utils.go
[failure] 288-288:
cannot use &node (value of type *"go.yaml.in/yaml/v3".Node) as *"gopkg.in/yaml.v3".Node value in argument to ExtractYAMLPositions
🪛 GitHub Check: Build (ubuntu-latest, linux)
pkg/utils/yaml_utils.go
[failure] 288-288:
cannot use &node (value of type *"go.yaml.in/yaml/v3".Node) as *"gopkg.in/yaml.v3".Node value in argument to ExtractYAMLPositions
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
🔇 Additional comments (9)
pkg/merge/merge_context_test.go (1)
280-585: Solid provenance test coverage.The test suite comprehensively validates provenance handling across edge cases including nil contexts, idempotency, cloning semantics, and type determination. Well done.
pkg/provenance/tree_renderer_test.go (1)
10-179: Thorough multiline YAML and path normalization tests.The test suite covers complex YAML structures including literal/folded scalars, nested maps, and edge cases. The path normalization tests validate component prefix stripping correctly.
pkg/merge/provenance_entry_test.go (1)
10-526: Excellent provenance entry test coverage.The test suite is comprehensive, covering all ProvenanceEntry operations with proper nil handling, equality checks, and hash validation. The constant verification is particularly thorough.
pkg/utils/yaml_position_test.go (1)
11-384: Comprehensive YAML position extraction tests.The test suite thoroughly covers position extraction across diverse YAML structures including nested maps, arrays, multiline content, and real-world Atmos configurations. Edge cases like nil nodes and disabled extraction are properly tested.
pkg/merge/provenance_storage_test.go (1)
11-566: Robust provenance storage tests with concurrency validation.The test suite comprehensively validates all storage operations including the critical concurrent access scenario. The concurrent test with 10 goroutines and 100 operations each properly validates thread safety.
pkg/utils/yaml_utils.go (1)
269-302: No action needed for nil positions
Go’s built-in len on a nil map returns 0, and callers only usepositionswhenTrackProvenanceis true andlen(positions)>0, so returning nil poses no risk.Likely an incorrect or invalid review comment.
pkg/merge/merge_with_provenance_test.go (1)
13-82: Solid coverage and assertions.Nice spread across scenarios; positions, depth, arrays, and multi-inputs are well exercised.
Also applies to: 84-141, 161-196, 198-235, 236-262, 264-316
pkg/merge/merge_with_provenance.go (1)
19-43: Looks good.Provenance recording is clear, depth-aware, and guarded behind the flag/context. perf.Track included.
Also applies to: 45-89, 91-124, 125-141
pkg/utils/jsonpath.go (1)
9-28: LGTM.Handy helpers with clear docs.
Also applies to: 30-56, 58-106, 108-136, 138-166, 167-184
osterman
changed the title
osterman
changed the title
describe component and describe stacks
- Update provenance legend: change "Defined at this level" to "Defined in parent stack" - Add LongString type that encodes as YAML folded scalars (>-) for long strings - Add WrapLongStrings function to automatically wrap long strings in YAML output - Integrate line wrapping in provenance renderer to prevent horizontal scrolling - Long values like atmos_component_description now wrap properly with provenance comments aligned This improves readability of provenance output by ensuring long string values are wrapped using proper YAML folded scalar syntax, keeping provenance comments aligned at the comment column and preventing the need for horizontal scrolling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
osterman
changed the title
describe component and describe stacks--provenance flag to describe component and describe stacks
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
pkg/utils/yaml_utils.go (1)
339-339: Resolve the yaml module path mismatch.The build failure at Line 339 occurs because
&nodeis typed as*go.yaml.in/yaml/v3.NodebutExtractYAMLPositionsexpects*gopkg.in/yaml.v3.Node. This indicates inconsistent imports across the codebase.As noted in the previous review, run the following script to identify all yaml.v3 imports and standardize them to
gopkg.in/yaml.v3:#!/bin/bash # Find all yaml.v3 import paths in Go files rg -nP '^\s*"(gopkg\.in/yaml\.v3|go\.yaml\.in/yaml/v3)"' --type=goUpdate all imports to use the same module path, update
go.mod, and rungo mod tidyto resolve the conflict.pkg/provenance/tree_renderer.go (1)
315-389: Array provenance missing for list items.
- key:entries still emitvars.items.name, but provenance is stored asvars.items[0].name,vars.items[1].name, etc., so inline comments vanish for arrays of maps. Please carry the per-level index into the key path when trimming the-prefix (and bump it afterwards) so the generated path matches storage.Apply this diff to thread the index into the composed path:
- if strings.HasPrefix(key, "- ") { - key = strings.TrimPrefix(key, "- ") - key = strings.TrimSpace(key) - } - - // Build full path - currentPath := key - if len(pathStack) > 0 { - currentPath = strings.Join(append(pathStack, key), ".") - } + var currentPath string + if strings.HasPrefix(key, "- ") { + key = strings.TrimSpace(strings.TrimPrefix(key, "- ")) + + if len(pathStack) > 0 { + parent := pathStack[len(pathStack)-1] + idx := 0 + if len(arrayIndexStack) > 0 { + idx = arrayIndexStack[len(arrayIndexStack)-1] + arrayIndexStack[len(arrayIndexStack)-1]++ + } else { + arrayIndexStack = append(arrayIndexStack, 1) + } + currentPath = fmt.Sprintf("%s[%d].%s", parent, idx, key) + } else { + currentPath = key + } + } else { + currentPath = key + if len(pathStack) > 0 { + currentPath = strings.Join(append(pathStack, key), ".") + } + }
🧹 Nitpick comments (1)
pkg/utils/yaml_utils.go (1)
165-177: Consider adding perf tracking to MarshalYAML.The coding guidelines require
defer perf.Track(...)in all public functions. While this method is very lightweight, adding tracking would align with the guidelines. You could passnilfor atmosConfig since it's not available in this context.Apply this diff if you choose to add tracking:
func (s LongString) MarshalYAML() (interface{}, error) { + defer perf.Track(nil, "utils.LongString.MarshalYAML")() + node := &yaml.Node{ Kind: yaml.ScalarNode, Style: yaml.FoldedStyle, // Use > style for folded scalar Value: string(s), } return node, nil }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
pkg/provenance/tree_renderer.go(1 hunks)pkg/utils/yaml_utils.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/utils/yaml_utils.gopkg/provenance/tree_renderer.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All Go comments must end with periods. Enforced by golangci-lint's godot.
Organize imports into three groups (stdlib, third-party, Atmos) separated by blank lines; sort alphabetically within each group; maintain existing aliases.
Add defer perf.Track(...) to all public functions and critical private functions, with a blank line after the call and package-prefixed name (e.g., "exec.ProcessComponent"). Use atmosConfig if available, otherwise nil.
Wrap all errors using static errors defined in errors/errors.go; never return dynamic errors directly.
Use the error wrapping pattern: fmt.Errorf("%w: details", errUtils.ErrStaticError, ...) with %w to preserve error chains and add context after the static error.
Bind environment variables with viper.BindEnv() and provide ATMOS_ alternatives for every env var (e.g., viper.BindEnv("ATMOS_GITHUB_TOKEN", "GITHUB_TOKEN")).
Ensure cross-platform compatibility: prefer SDKs over invoking binaries, use filepath.Join, os.PathSeparator/PathListSeparator, and runtime.GOOS when necessary.
Files:
pkg/utils/yaml_utils.gopkg/provenance/tree_renderer.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/utils/yaml_utils.gopkg/provenance/tree_renderer.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Do not use logging for UI output. Send prompts/status/progress and actionable errors to stderr; send data/results to stdout; keep logging for system/debug with structured fields.
Most text UI must go to stderr; only data/results go to stdout. Prefer utils.PrintfMessageToTUI for UI messages.
For non-standard execution paths, capture telemetry using telemetry.CaptureCmd(...) or telemetry.CaptureCmdString(...); never capture user data.
Files:
pkg/utils/yaml_utils.gopkg/provenance/tree_renderer.go
{internal/exec,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
All new configs must support Go templating using FuncMap() from internal/exec/template_funcs.go.
Files:
pkg/utils/yaml_utils.gopkg/provenance/tree_renderer.go
🧬 Code graph analysis (2)
pkg/utils/yaml_utils.go (3)
pkg/perf/perf.go (1)
Track(58-86)pkg/schema/schema.go (1)
AtmosConfiguration(27-64)pkg/utils/yaml_position.go (2)
PositionMap(14-14)ExtractYAMLPositions(19-27)
pkg/provenance/tree_renderer.go (6)
pkg/merge/merge_context.go (1)
MergeContext(13-33)pkg/perf/perf.go (1)
Track(58-86)pkg/merge/provenance_entry.go (6)
ProvenanceTypeComputed(22-22)ProvenanceEntry(30-49)ProvenanceTypeInline(16-16)ProvenanceTypeOverride(19-19)ProvenanceTypeImport(13-13)ProvenanceTypeDefault(25-25)pkg/ui/theme/colors.go (7)
ColorSelectedItem(22-22)ColorCyan(12-12)ColorDarkGray(15-15)ColorGreen(11-11)ColorYellow(17-17)ColorOrange(18-18)ColorRed(16-16)pkg/utils/yaml_utils.go (2)
ConvertToYAML(216-232)WrapLongStrings(181-214)pkg/utils/highlight_utils.go (1)
HighlightCodeWithConfig(81-122)
🪛 GitHub Check: Build (macos-latest, macos)
pkg/utils/yaml_utils.go
[failure] 339-339:
cannot use &node (value of type *"go.yaml.in/yaml/v3".Node) as *"gopkg.in/yaml.v3".Node value in argument to ExtractYAMLPositions
🪛 GitHub Check: Build (ubuntu-latest, linux)
pkg/utils/yaml_utils.go
[failure] 339-339:
cannot use &node (value of type *"go.yaml.in/yaml/v3".Node) as *"gopkg.in/yaml.v3".Node value in argument to ExtractYAMLPositions
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
🔇 Additional comments (2)
pkg/utils/yaml_utils.go (2)
179-214: LGTM on the long string wrapping logic.The function correctly handles recursive traversal of maps and slices, and the decision to wrap only single-line strings longer than maxLength is sound. The early return for maxLength <= 0 and the exclusion of multi-line strings are both sensible.
317-353: Approve the provenance tracking logic.The implementation correctly integrates position extraction when
atmosConfig.TrackProvenanceis enabled, and the function signature with PositionMap return aligns well with the PR objectives. Once the yaml module path issue is resolved, this function should work as intended.Verify that all call sites of this function properly handle the PositionMap return value and that the positions are correctly propagated to downstream provenance consumers.
aknysh
changed the title
--provenance flag to describe component--provenance flag to atmos describe component
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
pkg/merge/merge_with_provenance_test.go (2)
161-196: Consider verifying provenance details for both base and override.The test merges two inputs but only checks that provenance exists for the override value. Consider verifying:
- Override provenance has correct line/column from positions
- Base provenance exists with 0 line/column (no positions provided)
This would make the test more comprehensive and better document the expected behavior when merging inputs with partial position data.
354-359: Clarify scalar-array merge comment
Update the test’s comment to reflect that “merge” (WithSliceDeepCopy) preserves the base slice for scalar arrays (ignoring override), rather than saying it “behaves like replace.”pkg/provenance/text_utils.go (1)
244-250: Consider built-in max (Go 1.21+).If the project uses Go 1.21 or later, you can replace this with the built-in
maxfunction.-// max returns the maximum of two integers. -func max(a, b int) int { - if a > b { - return a - } - return b -}Then use
max(a, b)directly at line 163.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
docs/prd/import-provenance.md(1 hunks)pkg/merge/merge_with_provenance_test.go(1 hunks)pkg/provenance/provenance_helpers_test.go(1 hunks)pkg/provenance/text_utils.go(1 hunks)pkg/provenance/text_utils_test.go(1 hunks)pkg/provenance/tree_renderer.go(1 hunks)pkg/provenance/yaml_parser.go(1 hunks)pkg/provenance/yaml_parser_arrays_test.go(1 hunks)pkg/provenance/yaml_parser_multiline_test.go(1 hunks)website/docs/cli/commands/describe/describe-component.mdx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- website/docs/cli/commands/describe/describe-component.mdx
- docs/prd/import-provenance.md
- pkg/provenance/text_utils_test.go
- pkg/provenance/provenance_helpers_test.go
🧰 Additional context used
📓 Path-based instructions (5)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/provenance/text_utils.gopkg/provenance/yaml_parser_multiline_test.gopkg/provenance/yaml_parser.gopkg/provenance/tree_renderer.gopkg/merge/merge_with_provenance_test.gopkg/provenance/yaml_parser_arrays_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments in Go code must end with periods; enforced by golangci-lint's godot.
Organize imports into three groups (stdlib, 3rd-party, Atmos) separated by blank lines; sort alphabetically within each group; maintain existing import aliases.
Adddefer perf.Track()to all public functions and critical private functions, with a blank line after the call; use package-qualified names (e.g., "exec.ProcessComponent"); pass atmosConfig if available, else nil.
Error handling: wrap errors using static errors from errors/errors.go; prefer errors.Join for multiple errors; add context with fmt.Errorf("%w", ...); use errors.Is for checks; never compare error strings or return dynamic errors directly.
Always bind environment variables with viper.BindEnv; every env var must have an ATMOS_ alternative (e.g., viper.BindEnv("ATMOS_GITHUB_TOKEN", "GITHUB_TOKEN")).
Use structured logging for system/debug info and direct UI output to stderr; never use logging for UI elements; data/results intended for piping must go to stdout.
Most text UI must go to stderr (e.g., utils.PrintfMessageToTUI or fmt.Fprintf(os.Stderr,...)); only data/results go to stdout.
Ensure cross-platform compatibility: prefer SDKs over shelling out to binaries, use filepath/os utilities, and guard OS-specific logic with build constraints or runtime.GOOS when necessary.
Files:
pkg/provenance/text_utils.gopkg/provenance/yaml_parser_multiline_test.gopkg/provenance/yaml_parser.gopkg/provenance/tree_renderer.gopkg/merge/merge_with_provenance_test.gopkg/provenance/yaml_parser_arrays_test.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/provenance/text_utils.gopkg/provenance/yaml_parser.gopkg/provenance/tree_renderer.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Test quality: test behavior not implementation; avoid testing stubs or tautologies; use DI to avoid hard deps (os.Exit, CheckErrorPrintAndExit); remove always-skipped tests; table-driven tests must use real scenarios.
Always use t.Skipf with a clear reason instead of t.Skip; never call t.Skipf without a reason.
Co-locate test files alongside implementations using *_test.go naming.
Files:
pkg/provenance/yaml_parser_multiline_test.gopkg/merge/merge_with_provenance_test.gopkg/provenance/yaml_parser_arrays_test.go
pkg/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit tests under pkg/ as *_test.go files (co-located with implementation).
Files:
pkg/provenance/yaml_parser_multiline_test.gopkg/merge/merge_with_provenance_test.gopkg/provenance/yaml_parser_arrays_test.go
🧠 Learnings (2)
📚 Learning: 2025-10-06T02:10:05.491Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T02:10:05.491Z
Learning: Applies to **/*.go : All comments in Go code must end with periods; enforced by golangci-lint's godot.
Applied to files:
pkg/provenance/tree_renderer.go
📚 Learning: 2025-10-06T02:10:05.491Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T02:10:05.491Z
Learning: Applies to **/*.go : Add `defer perf.Track()` to all public functions and critical private functions, with a blank line after the call; use package-qualified names (e.g., "exec.ProcessComponent"); pass atmosConfig if available, else nil.
Applied to files:
pkg/provenance/tree_renderer.go
🧬 Code graph analysis (3)
pkg/provenance/yaml_parser.go (3)
pkg/perf/perf.go (1)
Track(58-86)pkg/provenance/tree_renderer.go (1)
YAMLLineInfo(56-60)pkg/utils/jsonpath.go (1)
AppendJSONPathIndex(50-56)
pkg/provenance/tree_renderer.go (7)
pkg/merge/merge_context.go (1)
MergeContext(16-36)pkg/perf/perf.go (1)
Track(58-86)pkg/merge/provenance_entry.go (6)
ProvenanceTypeComputed(23-23)ProvenanceEntry(31-50)ProvenanceTypeInline(17-17)ProvenanceTypeOverride(20-20)ProvenanceTypeImport(14-14)ProvenanceTypeDefault(26-26)pkg/ui/theme/colors.go (6)
ColorSelectedItem(22-22)ColorCyan(12-12)ColorGreen(11-11)ColorOrange(18-18)ColorDarkGray(15-15)ColorRed(16-16)pkg/logger/log.go (1)
Debug(24-26)pkg/utils/yaml_utils.go (4)
ConvertToYAML(216-232)WrapLongStrings(181-214)DefaultYAMLIndent(29-29)YAMLOptions(161-163)pkg/utils/highlight_utils.go (1)
HighlightCodeWithConfig(82-126)
pkg/merge/merge_with_provenance_test.go (6)
pkg/schema/schema.go (3)
AtmosConfiguration(27-65)Settings(684-688)AtmosSettings(251-271)pkg/merge/merge_context.go (2)
NewMergeContext(39-43)MergeContext(16-36)pkg/merge/merge_with_provenance.go (1)
MergeWithProvenance(21-52)pkg/utils/yaml_position.go (1)
PositionMap(14-14)pkg/merge/provenance_entry.go (2)
ProvenanceTypeInline(17-17)ProvenanceTypeImport(14-14)pkg/merge/merge.go (3)
ListMergeStrategyReplace(16-16)ListMergeStrategyAppend(17-17)ListMergeStrategyMerge(18-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: website-deploy-preview
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
🔇 Additional comments (13)
pkg/merge/merge_with_provenance_test.go (7)
13-37: LGTM!The test correctly verifies that provenance tracking is skipped when disabled both in config and context.
39-82: LGTM!Comprehensive test covering basic provenance tracking with proper verification of all provenance details.
84-121: LGTM!Test correctly verifies provenance tracking for nested map structures.
123-159: LGTM!Test properly verifies provenance tracking for array elements using index notation.
198-234: LGTM!Test correctly verifies import depth tracking with parent context chain and proper provenance type assignment.
236-262: LGTM!Test correctly verifies that provenance tracking works gracefully when position information is unavailable.
264-314: LGTM!Well-structured table-driven test with comprehensive coverage of import depth calculation scenarios.
pkg/provenance/text_utils.go (6)
1-11: LGTM! Clean setup.Package declaration and imports are properly structured. The
unicode/utf8import correctly addresses the rune-counting requirements flagged in previous reviews.
22-54: Nice fix on flush-at-limit.The logic correctly flushes when reaching
maxWidthbefore writing the next printable rune, addressing the previous overflow issue with whitespace-free lines. The ANSI sequence handling looks solid.
56-99: Solid rune-aware wrapping.The switch from byte length to
utf8.RuneCountInStringat line 64 correctly fixes the multi-byte character width issues from earlier reviews. The fallback tohardWrapWithANSIhandles edge cases cleanly.
101-131: ANSI-aware wrapping works as intended.The implementation correctly keeps ANSI escape sequences with their styled text by counting only printable runes for width calculations. This addresses the previous issue where sequences were split from their payload.
133-190: Padding guard prevents panic.The negative padding check at lines 152-154 correctly prevents the
strings.Repeatpanic whenleftWidthis small, addressing the previous issue. The two-column layout logic is clean.
223-242: stripANSI SGR-only stripping is sufficient
Provenance rendering exclusively uses lipgloss-generated SGR color/bold codes, so handling only sequences ending in “m” covers all current use cases.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
pkg/merge/merge.go (1)
186-194: List-merge strategy not passed to provenance path.The provenance-aware branch (line 190) calls
MergeWithProvenance(atmosConfig, inputs, context, context.Positions)without passing theappendSliceorsliceDeepCopyoptions derived fromListMergeStrategy. This means provenance-aware merges may not honor the configured strategy (replace/append/merge) while the standard path does.This duplicates the concern raised in the past review comment at lines 186-195. The issue remains: ensure
MergeWithProvenancerespectsListMergeStrategyby either accepting and applying these options or deriving them internally fromatmosConfig.Settings.ListMergeStrategy.pkg/merge/merge_with_provenance_test.go (1)
443-448: Still need an assertion for thecountfield.Same gap as last round: the comment says the base
countshould be dropped, but the test never checks it. Please assert the actual behavior (or fix the comment ifcountmust survive) so regressions can’t sneak in.assert.Equal(t, "override", merged["name"], "Override value should win") assert.Equal(t, "field", merged["extra"], "New field from override should be present") - // Note: "count" from base is NOT preserved with mergo's SliceDeepCopy - it replaces the element + // Note: "count" from base is NOT preserved with mergo's SliceDeepCopy - it replaces the element + _, hasCount := merged["count"] + assert.False(t, hasCount, "count should not be preserved when the element is replaced") // Verify provenance was still tracked. assert.True(t, ctx.HasProvenance("vars.items"), "Provenance should be tracked for vars.items")
🧹 Nitpick comments (7)
errors/errors.go (1)
39-39: LGTM! Clean consolidation.The declaration is correct and the move to the top-level block makes sense.
As a minor organizational note: you might consider grouping this with other command-related errors (
ErrNoCommandSpecified,ErrCommandNotFound) around lines 241-242 for better discoverability, though the current placement in the general section works fine.pkg/utils/highlight_utils.go (1)
85-89: Unify terminal-detection logic across both highlighters.
- Extract the stdout & stderr checks into a single helper (e.g.
isTerminal()) and invoke it from bothHighlightCodeandHighlightCodeWithConfig.- Optionally cache the stderr check if highlighting is performance-sensitive.
- Add a doc comment clarifying the intended TUI vs CLI behavior.
pkg/ui/theme/colors.go (2)
17-17: Update the comment to reflect actual usage.The comment says "Available for future use," but evidence from the previous review indicates
ColorYellowis already used inpkg/provenance/tree_renderer.go(lines 213, 304). Update the comment to describe its actual purpose or remove the "future use" clause if it's actively in use.Additionally, the parenthetical readability concern is valid—consider documenting this in user-facing theme docs or providing an adaptive color alternative for better accessibility across terminal backgrounds.
17-18: Remove or document unused ColorYellow
ColorYellow in pkg/ui/theme/colors.go:17 has no references; delete it or annotate its reserved-for-future-use status.internal/exec/describe_affected_test.go (1)
22-104: LGTM! Good test refactoring.The
createExpectedAffectedResultshelper eliminates duplication and makes test expectations more maintainable. The parameterization for template-processed vs unprocessed results is clear and correct.Minor observation: The parameter name
templatesProcessedmight be slightly confusing sincetruemeans templates were NOT processed (i.e., raw template strings remain). Consider a comment or rename for clarity:// createExpectedAffectedResults creates the expected affected results for testing. // When rawTemplates is true, returns results with unprocessed template strings ({{ ... }}). // When rawTemplates is false, returns results with processed/resolved values. func createExpectedAffectedResults(componentPath string, rawTemplates bool) []schema.Affected {cmd/describe_component_test.go (2)
58-67: Check flag Set errors in tests.
PersistentFlags().Setreturns an error; ignoring it can hide regressions when flags are renamed or removed. Please wrap each Set withrequire.NoError(t, ...)so the test fails fast if the flag lookup breaks.
97-104: Use t.TempDir for unique temp files.Hard-coding
filepath.Join(os.TempDir(), "test-provenance-output.yaml")can collide with parallel runs or leftovers from previous runs. Prefertmp := t.TempDir(); file := filepath.Join(tmp, "output.yaml")(oros.CreateTemp) so each test instance owns its workspace and cleanup is automatic.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (53)
cmd/describe_component.go(3 hunks)cmd/describe_component_test.go(2 hunks)docs/prd/import-provenance.md(1 hunks)errors/errors.go(1 hunks)go.mod(2 hunks)internal/exec/describe_affected_test.go(3 hunks)internal/exec/describe_component.go(8 hunks)internal/exec/describe_component_provenance_test.go(1 hunks)internal/exec/describe_component_test.go(2 hunks)internal/exec/describe_stacks.go(1 hunks)internal/exec/stack_processor_utils.go(13 hunks)internal/tui/templates/templater.go(1 hunks)internal/tui/utils/utils.go(1 hunks)pkg/hooks/store_cmd.go(1 hunks)pkg/merge/integration_test.go(1 hunks)pkg/merge/merge.go(1 hunks)pkg/merge/merge_context.go(5 hunks)pkg/merge/merge_context_test.go(1 hunks)pkg/merge/merge_with_provenance.go(1 hunks)pkg/merge/merge_with_provenance_test.go(1 hunks)pkg/merge/provenance_benchmark_test.go(1 hunks)pkg/merge/provenance_entry.go(1 hunks)pkg/merge/provenance_entry_test.go(1 hunks)pkg/merge/provenance_storage.go(1 hunks)pkg/merge/provenance_storage_test.go(1 hunks)pkg/merge/provenance_tracker.go(1 hunks)pkg/merge/provenance_tracker_test.go(1 hunks)pkg/provenance/data_transform.go(1 hunks)pkg/provenance/data_transform_test.go(1 hunks)pkg/provenance/import_provenance_test.go(1 hunks)pkg/provenance/provenance_helpers_test.go(1 hunks)pkg/provenance/text_utils.go(1 hunks)pkg/provenance/text_utils_test.go(1 hunks)pkg/provenance/tree_renderer.go(1 hunks)pkg/provenance/tree_renderer_test.go(1 hunks)pkg/provenance/yaml_indent_test.go(1 hunks)pkg/provenance/yaml_parser.go(1 hunks)pkg/provenance/yaml_parser_arrays_test.go(1 hunks)pkg/provenance/yaml_parser_multiline_test.go(1 hunks)pkg/schema/schema.go(1 hunks)pkg/ui/theme/colors.go(1 hunks)pkg/utils/highlight_utils.go(2 hunks)pkg/utils/jsonpath.go(1 hunks)pkg/utils/jsonpath_test.go(1 hunks)pkg/utils/yaml_include_by_extension.go(1 hunks)pkg/utils/yaml_include_by_extension_regression_test.go(1 hunks)pkg/utils/yaml_position.go(1 hunks)pkg/utils/yaml_position_test.go(1 hunks)pkg/utils/yaml_utils.go(3 hunks)pkg/utils/yq_utils.go(1 hunks)pkg/utils/yq_utils_test.go(1 hunks)website/docs/cli/commands/describe/describe-component.mdx(2 hunks)website/docs/cli/commands/describe/describe-stacks.mdx(1 hunks)
🧰 Additional context used
🧠 Learnings (14)
📚 Learning: 2025-10-06T02:10:05.491Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T02:10:05.491Z
Learning: Applies to **/*.go : All comments in Go code must end with periods; enforced by golangci-lint's godot.
Applied to files:
pkg/merge/merge_with_provenance.gopkg/provenance/tree_renderer.go
📚 Learning: 2025-05-22T19:58:32.988Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.
Applied to files:
cmd/describe_component_test.go
📚 Learning: 2025-06-07T19:27:40.807Z
Learnt from: samtholiya
PR: cloudposse/atmos#1266
File: pkg/utils/highlight_utils.go:0-0
Timestamp: 2025-06-07T19:27:40.807Z
Learning: In pkg/utils/highlight_utils.go, the global variable `isTermPresent` that caches terminal detection at package initialization is an intentional design choice and should not be changed to per-call detection.
Applied to files:
pkg/utils/highlight_utils.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
PR: cloudposse/atmos#1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/utils/yaml_utils.go
📚 Learning: 2025-10-06T02:10:05.491Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T02:10:05.491Z
Learning: Applies to pkg/ui/theme/**/*.go : Use consistent Atmos theme colors from pkg/ui/theme/colors.go via theme.Styles/theme.Colors for UI elements.
Applied to files:
pkg/ui/theme/colors.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
pkg/ui/theme/colors.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
pkg/merge/provenance_entry_test.gopkg/utils/jsonpath_test.go
📚 Learning: 2025-10-06T02:10:05.491Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T02:10:05.491Z
Learning: Applies to **/*.go : Add `defer perf.Track()` to all public functions and critical private functions, with a blank line after the call; use package-qualified names (e.g., "exec.ProcessComponent"); pass atmosConfig if available, else nil.
Applied to files:
pkg/provenance/tree_renderer.gointernal/exec/stack_processor_utils.gointernal/exec/describe_component.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
PR: cloudposse/atmos#764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
internal/exec/describe_stacks.go
📚 Learning: 2025-10-06T02:10:05.491Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T02:10:05.491Z
Learning: Applies to pkg/**/*_test.go : Place unit tests under pkg/ as *_test.go files (co-located with implementation).
Applied to files:
pkg/utils/jsonpath_test.go
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
PR: cloudposse/atmos#795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
internal/exec/stack_processor_utils.go
📚 Learning: 2024-11-10T18:37:10.032Z
Learnt from: osterman
PR: cloudposse/atmos#768
File: internal/exec/vendor_component_utils.go:354-360
Timestamp: 2024-11-10T18:37:10.032Z
Learning: In the vendoring process, a TTY can exist without being interactive. If the process does not prompt the user, we should not require interactive mode to display the TUI. The `CheckTTYSupport` function should check TTY support on stdout rather than stdin.
Applied to files:
internal/exec/describe_component.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
PR: cloudposse/atmos#1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
internal/exec/describe_component.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
internal/exec/describe_component.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Acceptance Tests (windows-latest, windows)
- GitHub Check: Acceptance Tests (macos-latest, macos)
- GitHub Check: Acceptance Tests (ubuntu-latest, linux)
- GitHub Check: Summary
🔇 Additional comments (59)
pkg/utils/highlight_utils.go (1)
12-16: LGTM! Clean migration to chroma v2.The import updates are correct and consistent with the chroma v2 API.
internal/tui/utils/utils.go (1)
9-15: Confirm chroma v2 migration and review changelog for behavioral changes.
All chroma imports across the repository now use v2; ensure no API or output changes in v2 affect syntax highlighting.pkg/utils/yq_utils.go (1)
14-14: LGTM! Import path standardization.The switch to the canonical
gopkg.in/yaml.v3import with a local alias is correct and maintains consistency across the codebase.pkg/utils/yaml_include_by_extension_regression_test.go (1)
9-9: LGTM! Consistent import path update.The import change aligns with the standardization across the codebase.
internal/exec/describe_stacks.go (2)
68-68: LGTM! More specific performance tracking label.The updated label
exec.DescribeStacksExec.Executeprovides better granularity for performance monitoring.
110-121: Verify removal of performance tracking.The
perf.Trackdefer call appears to have been removed fromExecuteDescribeStacks. This function is substantial (800+ lines) and processes stack manifests extensively. Removing performance tracking here reduces observability for this critical path.Confirm whether the removal was intentional. If so, consider re-adding tracking to maintain visibility into performance characteristics:
func ExecuteDescribeStacks( atmosConfig *schema.AtmosConfiguration, filterByStack string, components []string, componentTypes []string, sections []string, ignoreMissingFiles bool, processTemplates bool, processYamlFunctions bool, includeEmptyStacks bool, skip []string, ) (map[string]any, error) { + defer perf.Track(atmosConfig, "exec.ExecuteDescribeStacks")() + stacksMap, _, err := FindStacksMap(atmosConfig, ignoreMissingFiles)pkg/utils/yaml_include_by_extension.go (1)
10-10: LGTM! Consistent import standardization.The canonical import path aligns with the broader YAML library standardization effort.
internal/tui/templates/templater.go (1)
10-12: LGTM! Explicit dependency declarations.Making these direct imports explicit improves clarity and aligns with Go best practices, especially since the file directly uses types from these packages.
pkg/utils/yq_utils_test.go (1)
7-7: LGTM! Test import alignment.The import change maintains consistency with production code changes.
go.mod (2)
194-194: LGTM! Transitive dependency update.The regexp2 patch bump from v1.11.0 to v1.11.5 is a transitive dependency of chroma with no known breaking changes.
15-15: Verify Chroma v2 upgrade compatibility. In pkg/utils/highlight_utils.go, review getLexer (line 129) and getFormatter (line 153)—exercise syntax highlighting across languages and output formats to confirm no breaking changes in lexers.Get, formatters.TTY256 or style defaults in v2.20.0.internal/exec/describe_affected_test.go (2)
311-311: LGTM! Cleaner test expectations.Using the helper function improves readability and maintainability.
333-333: LGTM! Consistent helper usage.The helper provides consistent expected data across test scenarios.
pkg/provenance/text_utils.go (1)
1-250: LGTM!The ANSI-aware text utilities look solid after the fixes from previous reviews. The rune-safe wrapping, ANSI sequence preservation, and panic prevention all work correctly now. The implementation handles the core use cases well.
pkg/provenance/text_utils_test.go (1)
1-340: Excellent test coverage!The test suite thoroughly validates ANSI handling, wrapping behavior, and layout rendering. Good use of table-driven tests and edge case coverage.
pkg/merge/provenance_entry.go (1)
1-148: LGTM!The provenance entry model is well-structured with proper deterministic hashing via
json.Marshal. The 8-byte hash truncation is appropriate for configuration change detection.pkg/merge/provenance_storage.go (1)
1-174: LGTM!The provenance storage is well-implemented with proper thread-safety, defensive copies, and nil-safety. Good use of read/write locks and sorted output for consistency.
pkg/utils/yaml_utils.go (2)
165-214: Clean implementation of long string wrapping.The
LongStringtype andWrapLongStringsfunction handle YAML folded scalars nicely. The recursive walk creates new data structures (no side effects), and the logic correctly identifies long single-line strings.
317-353: Good integration of provenance tracking.The new
UnmarshalYAMLFromFileWithPositionsfunction conditionally extracts position data based onatmosConfig.TrackProvenance, which keeps the overhead minimal when provenance is disabled.website/docs/cli/commands/describe/describe-stacks.mdx (1)
93-95: Clear documentation note.The provenance tracking note appropriately guides users to the component-level command for provenance details.
pkg/merge/merge_context_test.go (1)
281-583: Comprehensive provenance test coverage.The provenance tests thoroughly validate enabling, recording, retrieval, cloning, and type inference. Good coverage of nil-safety and the shared storage semantics via
WithFile.cmd/describe_component.go (1)
64-80: Clean flag integration.The provenance flag handling follows the established pattern and integrates cleanly with the existing command structure.
website/docs/cli/commands/describe/describe-component.mdx (2)
62-63: LGTM!The example usage clearly demonstrates the
--provenanceflag and follows the established pattern for other command examples.
96-121: Excellent provenance documentation.The flag documentation comprehensively covers all output formats (YAML TTY, YAML non-TTY, JSON) with clear explanations of symbols, depth indicators, and color coding. The distinction between TTY and non-TTY behavior is well-articulated, and the JSON structure is clearly defined.
pkg/schema/schema.go (1)
64-64: LGTM!The
TrackProvenancefield is cleanly integrated into theAtmosConfigurationstruct with proper tags for YAML/JSON/mapstructure serialization. The field name is clear and the optional nature (omitempty) is appropriate for a feature flag.docs/prd/import-provenance.md (1)
1-601: Excellent PRD documentation.This comprehensive PRD provides clear context for the import provenance tracking system, covering architecture, design decisions, implementation phases, and testing strategy. The document effectively explains the rationale for key decisions (inline YAML comments, ProvenanceTracker interface, data structure purity) and includes practical use cases. All previous review comments have been addressed.
pkg/utils/jsonpath_test.go (1)
1-477: Comprehensive test coverage for JSONPath utilities.This test file provides excellent coverage for all JSONPath helper functions with clear table-driven tests. The inclusion of round-trip tests (lines 409-436) and parent-child relationship tests (lines 438-477) validates the correctness of bidirectional operations. Edge cases like empty strings, invalid formats, and array indices are well-covered.
pkg/provenance/yaml_parser_multiline_test.go (2)
10-71: Thorough multiline scalar indicator tests.The test covers all valid YAML multiline scalar indicators (literal
|, folded>, with chomping and indent indicators) plus comprehensive edge cases and invalid formats. The test cases are well-named and clearly document expected behavior.
74-131: Context test validates multiline scalar handling.This test verifies that
buildYAMLPathMapcorrectly handles multiline YAML scalars by identifying key lines and continuation lines. The note at line 124 documents a known limitation where lines starting with#inside multiline scalars are filtered as comments—an acceptable trade-off for the current implementation.pkg/utils/yaml_position_test.go (1)
1-384: Comprehensive YAML position extraction tests.This test file thoroughly validates YAML position extraction across a wide range of scenarios: simple maps, nested structures, arrays, arrays of maps, complex configurations, and edge cases (disabled extraction, nil nodes, empty structures). The inclusion of a real-world Atmos stack configuration test (lines 345-384) ensures practical applicability. The ordering assertions verify parent-child relationships, and all tests validate both line and column precision.
pkg/provenance/tree_renderer_test.go (3)
10-130: Comprehensive multiline YAML path mapping tests.The table-driven tests cover all major YAML multiline scenarios: literal scalars (
|), folded scalars (>), chomping indicators (-,+), and nested multiline values. The tests validate thatbuildYAMLPathMapcorrectly identifies key lines vs continuation lines, which is essential for accurate provenance rendering.
132-171: Path normalization tests cover key scenarios.The tests validate that
normalizeProvenancePathcorrectly stripscomponents.terraform.*andterraform.*prefixes while handling edge cases like already-normalized paths and paths that reduce to empty strings. The test cases are clear and cover the expected usage patterns.
173-179: Default comment column test is reasonable.The test validates that
getCommentColumnreturns a sensible default value in non-TTY environments (which includes test runs). The range check (40-200) ensures the column position is neither too small nor unreasonably large.internal/exec/stack_processor_utils.go (7)
35-44: LGTM!Clean per-stack provenance storage with proper mutex protection. The dual storage (per-stack + deprecated last-context) maintains backward compatibility while enabling the new per-stack workflow.
46-101: LGTM!All public context management functions properly instrumented with
perf.Trackand use correct mutex patterns. Clean API surface for per-stack provenance storage.
147-153: LGTM!Excellent fix - each goroutine now creates its own
MergeContext, eliminating the data race. Clean per-goroutine provenance tracking.
213-217: LGTM!Proper per-stack context storage with backward-compatible mirroring. The note about potential race on
lastMergeContextis accurate and acceptable for the deprecated path.
278-309: LGTM!Clean refactor that adds provenance support to single-file operations while maintaining backward compatibility. Context creation and storage are properly handled.
540-569: LGTM!Import provenance recording correctly uses
GetImportDepth()for consistent depth calculation. Clean key format and first-occurrence logic.
725-747: LGTM!Metadata provenance recording also uses
GetImportDepth()for consistent depth tracking. Good pattern matching with the import provenance logic.internal/exec/describe_component.go (8)
80-120: LGTM!Clean routing to provenance-enabled path when requested. Proper context retrieval, computed-field filtering, and backward-compatible fallback to standard execution.
133-140: LGTM!Proper validation before provenance rendering - checks context availability and data type. Clean error message if validation fails.
221-248: LGTM!Helper functions correctly use raw file writes (not YAML marshaling) for provenance output. Clean separation of file vs. stdout handling.
250-267: LGTM!Safe type assertion with proper
okcheck prevents panics. Clean handling of both[]anyand[]stringcases with graceful skip of non-string entries.
269-362: LGTM!Well-structured import provenance helpers with clear fallback logic. Clean separation between metadata lookups, YAML position enhancement, and all-paths search.
364-426: LGTM!Clean component type detection with proper fallback chain (Terraform → Helmfile → Packer). Well-structured parameter types and processing helpers.
429-486: LGTM!Comprehensive context-aware implementation with proper cleanup (
ClearMergeContexts), type detection, per-stack context retrieval with fallback, and import provenance recording.
488-515: LGTM!Clean filtering of computed fields - keeps only stack-defined fields (vars, settings, env, backend, metadata, overrides, providers, imports) for provenance display.
pkg/provenance/tree_renderer.go (11)
1-40: LGTM!Clean package setup with well-organized constants and clear documentation. Good separation of rendering constants and string constants.
42-60: LGTM!Clean type definitions with proper doc comments ending in periods. Types clearly represent provenance rendering concepts.
62-84: LGTM!Clean tree rendering with proper null/disabled checks and clear separation of build and render phases.
86-135: LGTM!Proper
perf.Trackinstrumentation and clean file grouping logic. Takes first (most recent) entry and produces sorted results.
137-224: LGTM!Clean tree rendering with box drawing characters, symbol mapping, and lipgloss styling. Good color coding by symbol type.
226-259: LGTM!Clean side-by-side rendering with panic recovery, highlighting with fallback, and proper YAML conversion.
261-283: LGTM!Clean provenance lookup with path normalization and first-match logic.
285-367: LGTM!Clean provenance comment formatting with depth-based color coding and terminal-aware column calculation. Good lipgloss integration.
375-409: LGTM!Proper
perf.Trackinstrumentation and graceful highlight failure handling. Clean YAML preparation with import renaming, filtering, wrapping, and indentation.
411-498: LGTM!Clean line rendering with ANSI stripping for length calculation, proper padding/wrapping, legend rendering, and smart skip logic for continuations and non-key lines.
500-545: LGTM!Comprehensive inline provenance rendering with proper
perf.Track, panic recovery, legend rendering, YAML preparation, and line-by-line processing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/merge/merge_with_provenance_test.go (1)
191-191: Consider adding a clarifying comment.Since
positionsonly contains data foroverride.yaml(not the base), a brief comment explaining this would help future readers understand why provenance for base values isn't being tracked in this particular test.ctx.EnableProvenance() atmosConfig := &schema.AtmosConfiguration{ TrackProvenance: true, } + // Note: positions only contains data for override.yaml, not base. _, err := MergeWithProvenance(atmosConfig, []map[string]any{base, override}, ctx, positions)pkg/provenance/yaml_parser_arrays_test.go (1)
132-147: Consider adding assertions to the debug test.This test only logs the path map without verifying behavior. While debug output can be helpful during development, tests should assert expected outcomes.
Either add assertions like the other tests or convert this to a test helper function if the logging is genuinely useful for future debugging:
func TestSimpleArrayDebug(t *testing.T) { yaml := `vars: items: - name: widget color: blue` lines := strings.Split(yaml, "\n") pathMap := buildYAMLPathMap(lines) - t.Log("\n=== PATH MAP ===") - for i := 0; i < len(lines); i++ { - if info, ok := pathMap[i]; ok { - t.Logf("Line %d: %q -> Path: %q", i, strings.TrimSpace(lines[i]), info.Path) - } - } + // Verify array element paths are indexed + if info, ok := pathMap[2]; ok { + assert.Equal(t, "vars.items[0].name", info.Path) + assert.True(t, info.IsKeyLine) + } else { + t.Fatal("Line 2 not found in path map") + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
pkg/merge/merge_with_provenance_test.go(1 hunks)pkg/provenance/yaml_parser.go(1 hunks)pkg/provenance/yaml_parser_arrays_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/provenance/yaml_parser_arrays_test.gopkg/provenance/yaml_parser.gopkg/merge/merge_with_provenance_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Test quality: test behavior not implementation; avoid testing stubs or tautologies; use DI to avoid hard deps (os.Exit, CheckErrorPrintAndExit); remove always-skipped tests; table-driven tests must use real scenarios.
Always use t.Skipf with a clear reason instead of t.Skip; never call t.Skipf without a reason.
Co-locate test files alongside implementations using *_test.go naming.
Files:
pkg/provenance/yaml_parser_arrays_test.gopkg/merge/merge_with_provenance_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments in Go code must end with periods; enforced by golangci-lint's godot.
Organize imports into three groups (stdlib, 3rd-party, Atmos) separated by blank lines; sort alphabetically within each group; maintain existing import aliases.
Adddefer perf.Track()to all public functions and critical private functions, with a blank line after the call; use package-qualified names (e.g., "exec.ProcessComponent"); pass atmosConfig if available, else nil.
Error handling: wrap errors using static errors from errors/errors.go; prefer errors.Join for multiple errors; add context with fmt.Errorf("%w", ...); use errors.Is for checks; never compare error strings or return dynamic errors directly.
Always bind environment variables with viper.BindEnv; every env var must have an ATMOS_ alternative (e.g., viper.BindEnv("ATMOS_GITHUB_TOKEN", "GITHUB_TOKEN")).
Use structured logging for system/debug info and direct UI output to stderr; never use logging for UI elements; data/results intended for piping must go to stdout.
Most text UI must go to stderr (e.g., utils.PrintfMessageToTUI or fmt.Fprintf(os.Stderr,...)); only data/results go to stdout.
Ensure cross-platform compatibility: prefer SDKs over shelling out to binaries, use filepath/os utilities, and guard OS-specific logic with build constraints or runtime.GOOS when necessary.
Files:
pkg/provenance/yaml_parser_arrays_test.gopkg/provenance/yaml_parser.gopkg/merge/merge_with_provenance_test.go
pkg/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit tests under pkg/ as *_test.go files (co-located with implementation).
Files:
pkg/provenance/yaml_parser_arrays_test.gopkg/merge/merge_with_provenance_test.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/provenance/yaml_parser.go
🧬 Code graph analysis (2)
pkg/provenance/yaml_parser.go (3)
pkg/perf/perf.go (1)
Track(58-86)pkg/provenance/tree_renderer.go (1)
YAMLLineInfo(56-60)pkg/utils/jsonpath.go (1)
AppendJSONPathIndex(50-56)
pkg/merge/merge_with_provenance_test.go (6)
pkg/schema/schema.go (3)
AtmosConfiguration(27-65)Settings(684-688)AtmosSettings(251-271)pkg/merge/merge_context.go (2)
NewMergeContext(39-43)MergeContext(16-36)pkg/merge/merge_with_provenance.go (1)
MergeWithProvenance(21-52)pkg/utils/yaml_position.go (1)
PositionMap(14-14)pkg/merge/provenance_entry.go (2)
ProvenanceTypeInline(17-17)ProvenanceTypeImport(14-14)pkg/merge/merge.go (3)
ListMergeStrategyReplace(16-16)ListMergeStrategyAppend(17-17)ListMergeStrategyMerge(18-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (20)
pkg/merge/merge_with_provenance_test.go (3)
447-448: Past review comment addressed.The comment and assertion for the
countfield behavior have been updated correctly. The comment now accurately states thatcountIS preserved with mergo's SliceDeepCopy, and the assertion on line 448 verifies this behavior.
13-453: Comprehensive test coverage.The test suite thoroughly covers provenance tracking scenarios including disabled provenance, simple/nested structures, arrays, multiple inputs, import depth, missing positions, and list merge strategies. The use of table-driven tests for
TestGetImportDepthand subtests inTestMergeWithProvenance_HonorsListMergeStrategyfollows Go best practices.
354-359: <!-- [scratchpad]
[task review_merge_with_provenance_test.inspection_1/1]
[observations]
- The test file under review is pkg/merge/merge_with_provenance_test.go.
- Relevant snippet shows two test cases:
- Before line 354: expectedTags: []any{"base-tag-1", "base-tag-2", "override-tag-1"}.
- Test at lines 354-359: expectedTags: []any{"base-tag-1", "base-tag-2"}.
- Merge implementation uses mergo.WithSliceDeepCopy for ListMergeStrategyMerge, which makes slices replace behavior (src overrides dst).
[analysis]- If override map provides a tags slice, merge strategy should replace base tags with override tags.
- The first test case likely has override tags; the second test case's override map might omit tags, leading to base tags preserved.
[pending]- Confirm whether the override map in the second test case omits the tags field.
[actions]- Print the test case definitions, focusing on the maps for base and override entries in the "merge strategy - scalars behave like replace" scenario.
[/scratchpad]pkg/provenance/yaml_parser.go (10)
1-13: LGTM!Package declaration, imports, and constants are properly structured. Imports follow the three-group convention (stdlib → third-party → Atmos), and the YAML array prefix constant is clearly defined.
15-36: Normalization logic is sound.The function correctly strips common component prefixes while preserving the essential path structure. Performance tracking is properly applied with nil atmosConfig.
38-63: Multiline indicator detection is comprehensive.Correctly handles literal (
|) and folded (>) styles with optional chomping (+/-) and indent indicators (0-9). Based on learnings from past reviews, this now covers all valid YAML multiline scalar indicators.
65-85: Helper functions are clean and focused.
extractYAMLKeyproperly strips array prefixes and colons.buildYAMLPathcorrectly joins path components with the separator. Both handle edge cases appropriately.
87-102: Array index tracking is correct.The function initializes and increments indices properly, returning both the current index and the updated stack for the next element.
104-116: Stack unwinding logic handles root-level edge cases.Line 111's condition
(indent > 0 || len(pathStack) > 0)correctly preserves the root counter while popping nested scopes, addressing the regression mentioned in past reviews.
118-143: Array item handling supports both root and nested contexts.Lines 123-125 correctly handle root-level sequences with
utils.AppendJSONPathIndex("", arrayIndex), while lines 129-132 build full prefixes for nested arrays. This addresses the path prefix issue from past reviews.
173-242: Root-level array-of-maps indexing is correct.
buildRootArrayElementPath(lines 173-194) properly constructs[i].keypaths, andbuildArrayElementPath(lines 196-242) delegates to it whenpathStackis empty. This fixes the root-level indexing issue mentioned in past reviews.
244-303: Key line processing handles all array scenarios.Lines 253-257 delegate to
buildArrayElementPathfor array elements regardless of nesting level, removing thelen(pathStack) > 0guard mentioned in past reviews. Lines 289-292 correctly track indent for root-level array elements.
383-400: Main entry point is well-structured.Performance tracking is correctly applied. The line-by-line loop with persistent state cleanly models YAML structure. The implementation addresses all array indexing, root-level, and multiline issues flagged in previous reviews.
pkg/provenance/yaml_parser_arrays_test.go (7)
10-54: Solid test for array-of-maps indexing.Validates that
- name: widgetproducesvars.items[0].nameinstead of the incorrectvars.items.name. Clear assertions and error messages.
56-97: Nested array indexing is thoroughly tested.Confirms double-indexed paths like
config.servers[0].ports[0].port. Good coverage of the nesting scenarios.
99-130: Arrays with nested objects are validated.Tests that nested object fields maintain the correct array index through multiple levels. Well-structured.
149-191: Root-level scalar arrays are properly tested.Validates that
- item1produces[0]rather than dropping the index. The debug logging is helpful context.
193-242: Root-level array-of-maps coverage is thorough.Confirms that
- key: value1produces[0].keyat the document root. Clear assertions on both the leading and nested keys.
244-290: Regression test for nested scalar arrays is valuable.Lines 245-246 explain this tests the bug where
handleArrayItemLineonly used the last stack element. Verifies the full prefix is now included.
292-364: Regression test for root array counters is critical.Lines 293-294 document the bug where
arrayIndexStackwasn't popped correctly at root level, causing siblings to collapse onto[0]. This test ensures indices advance correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
pkg/utils/jsonpath_test.go (4)
122-161: Consider testing negative indices.The function accepts an
intparameter, so negative indices are technically possible. Consider adding a test case to document expected behavior (e.g., whetherAppendJSONPathIndex("items", -1)produces"items[-1]"or is considered invalid).Example test case:
+ { + name: "negative index", + basePath: "items", + index: -1, + expected: "items[-1]", + },
163-217: Consider testing malformed bracket handling.The implementation in
pkg/utils/jsonpath.go(lines 78-120) has explicit handling for malformed paths with unclosed brackets. Adding a test case would document this behavior and prevent regressions.Example test case:
+ { + name: "malformed path - unclosed bracket", + path: "vars[0", + expected: []string{"vars[0"}, + },
219-279: Consider testing negative indices.Similar to
TestAppendJSONPathIndex, adding a test case for negative indices would document expected behavior (e.g., whether"[-1]"is considered a valid index).Example test case:
+ { + name: "negative index", + component: "[-1]", + expectedIndex: -1, + expectedOk: true, + },
281-315: Consider testing negative indices.Consistent with the prior functions, adding a negative index test case would complete the edge case coverage.
Example test case:
+ { + name: "negative index", + component: "[-1]", + expected: true, + },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
pkg/merge/merge_with_provenance.go(1 hunks)pkg/utils/jsonpath.go(1 hunks)pkg/utils/jsonpath_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/merge/merge_with_provenance.go
- pkg/utils/jsonpath.go
🧰 Additional context used
📓 Path-based instructions (4)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/utils/jsonpath_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Test quality: test behavior not implementation; avoid testing stubs or tautologies; use DI to avoid hard deps (os.Exit, CheckErrorPrintAndExit); remove always-skipped tests; table-driven tests must use real scenarios.
Always use t.Skipf with a clear reason instead of t.Skip; never call t.Skipf without a reason.
Co-locate test files alongside implementations using *_test.go naming.
Files:
pkg/utils/jsonpath_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments in Go code must end with periods; enforced by golangci-lint's godot.
Organize imports into three groups (stdlib, 3rd-party, Atmos) separated by blank lines; sort alphabetically within each group; maintain existing import aliases.
Adddefer perf.Track()to all public functions and critical private functions, with a blank line after the call; use package-qualified names (e.g., "exec.ProcessComponent"); pass atmosConfig if available, else nil.
Error handling: wrap errors using static errors from errors/errors.go; prefer errors.Join for multiple errors; add context with fmt.Errorf("%w", ...); use errors.Is for checks; never compare error strings or return dynamic errors directly.
Always bind environment variables with viper.BindEnv; every env var must have an ATMOS_ alternative (e.g., viper.BindEnv("ATMOS_GITHUB_TOKEN", "GITHUB_TOKEN")).
Use structured logging for system/debug info and direct UI output to stderr; never use logging for UI elements; data/results intended for piping must go to stdout.
Most text UI must go to stderr (e.g., utils.PrintfMessageToTUI or fmt.Fprintf(os.Stderr,...)); only data/results go to stdout.
Ensure cross-platform compatibility: prefer SDKs over shelling out to binaries, use filepath/os utilities, and guard OS-specific logic with build constraints or runtime.GOOS when necessary.
Files:
pkg/utils/jsonpath_test.go
pkg/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit tests under pkg/ as *_test.go files (co-located with implementation).
Files:
pkg/utils/jsonpath_test.go
🧠 Learnings (3)
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
pkg/utils/jsonpath_test.go
📚 Learning: 2025-10-06T02:10:05.491Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T02:10:05.491Z
Learning: Applies to pkg/**/*_test.go : Place unit tests under pkg/ as *_test.go files (co-located with implementation).
Applied to files:
pkg/utils/jsonpath_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Use table-driven tests for multiple scenarios
Applied to files:
pkg/utils/jsonpath_test.go
🧬 Code graph analysis (1)
pkg/utils/jsonpath_test.go (1)
pkg/utils/jsonpath.go (8)
BuildJSONPath(15-43)AppendJSONPathKey(50-58)AppendJSONPathIndex(65-71)SplitJSONPath(79-121)ParseJSONPathIndex(129-141)IsJSONPathIndex(148-151)GetJSONPathParent(159-180)GetJSONPathLeaf(188-199)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: website-deploy-preview
- GitHub Check: Run pre-commit hooks
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (7)
pkg/utils/jsonpath_test.go (7)
9-73: Comprehensive coverage for BuildJSONPath.Excellent table-driven test covering empty components, single/nested paths, array indices, and mixed constructs. Edge cases like filtering empty components are well-tested.
75-120: Solid coverage for AppendJSONPathKey.Tests handle empty paths, empty keys, and nested combinations well.
317-371: Thorough coverage for GetJSONPathParent.Tests handle empty paths, root levels, nested structures, and array indices well. The edge cases are well-represented.
373-427: Excellent coverage for GetJSONPathLeaf.Tests handle all major path structures including empty paths, single components, nested paths, and array indices.
429-472: Solid round-trip verification.Testing that
BuildJSONPathandSplitJSONPathare inverse operations is a smart approach to catching inconsistencies.
474-513: Good parent-child relationship verification.Reconstructing the original path from parent and leaf components validates the consistency of
GetJSONPathParentandGetJSONPathLeaf. The switch-based reconstruction logic correctly handles root paths, array indices, and regular keys.
430-430: Add period to comment.Comments must end with periods per coding guidelines (godot rule).
Apply this diff:
- // Test that building and splitting are inverse operations. + // Test that building and splitting are inverse operations.Wait, the comment already has a period. Let me re-check... Yes, line 430 has
// Test that building and splitting are inverse operations.which ends with a period. This is correct. I was mistaken.
* feat: add --show-provenance flag to describe component and describe stacks Add provenance tracking display that shows the source file and line number for each configuration value in component and stack descriptions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * docs: revert broken provenance implementation, add comprehensive PRD - Reverted incomplete provenance implementation from pkg/ui/provenance/ - Removed --show-provenance flags from describe component and describe stacks commands - Created comprehensive PRD for proper line-level provenance system at docs/prd/line-level-provenance.md The initial provenance implementation had fundamental limitations: - Only tracked top-level section keys (vars.*, settings.*), not nested values - Two-column rendering was broken and couldn't align with complex YAML structures - No line number tracking, only file paths - Not reusable across different Atmos config types (stacks, atmos.yaml, vendor, workflows) - Output polluted with 750+ lines of raw sources data - Tests didn't catch critical bugs - PRD: docs/prd/line-level-provenance.md - Extends existing MergeContext system (pkg/merge/merge_context.go) - Uses gopkg.in/yaml.v3 Node API for line tracking - Estimated 3-4 weeks for proper implementation - Backward compatible deprecation of current sources system 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * docs: rename PRD from line-level-provenance to import-provenance Renamed PRD to better reflect that we're tracking import/inheritance chains rather than just line-level granularity. 'Import provenance' more accurately describes tracking where values come from through the import system. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * feat: complete provenance tracking implementation for nested imports - Implement MergeWithProvenance function to track provenance during YAML merging - Add provenance storage and entry management with thread-safe operations - Create tree renderer for inline provenance visualization with depth indicators - Add YAML position extraction utilities for accurate line/column tracking - Fix nested import provenance tracking in stack processor - Record provenance metadata using merge context's current file - Handle []any type for imports array in describe component output - Add comprehensive test suite for provenance tracking functionality - Support JSONPath addressing for nested configuration values - Add benchmarks for provenance operations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) * [autofix.ci] apply automated fixes (attempt 3/3) * feat: improve provenance display with line wrapping and updated legend - Update provenance legend: change "Defined at this level" to "Defined in parent stack" - Add LongString type that encodes as YAML folded scalars (>-) for long strings - Add WrapLongStrings function to automatically wrap long strings in YAML output - Integrate line wrapping in provenance renderer to prevent horizontal scrolling - Long values like atmos_component_description now wrap properly with provenance comments aligned This improves readability of provenance output by ensuring long string values are wrapped using proper YAML folded scalar syntax, keeping provenance comments aligned at the comment column and preventing the need for horizontal scrolling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) * [autofix.ci] apply automated fixes (attempt 3/3) * fix: address CodeRabbit review feedback for provenance tracking - Fix file output with --provenance flag by writing raw YAML to file - Add perf.Track instrumentation to merge context helper functions - Replace manual depth calculations with GetImportDepth() method - Fix goroutine data race by using per-goroutine MergeContext - Fix linting issues (godot comment capitalization, gosec file permissions) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) * [autofix.ci] apply automated fixes (attempt 3/3) * feat: add ProvenanceTracker interface and describe stacks provenance - Create generic ProvenanceTracker interface for extensibility - Add --provenance flag to describe stacks command - Update MergeContext to document ProvenanceTracker implementation - Add comprehensive unit tests for interface with mock implementation - Update PRD to reflect actual implementation status and design decisions - Document display format rationale (valid YAML output) - Document symbol and color scheme with Unicode codes - Document backward compatibility with sources system 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) * [autofix.ci] apply automated fixes (attempt 3/3) * fix: resolve yaml package type conflicts and address code review - Add explicit yaml alias to gopkg.in/yaml.v3 imports to prevent type conflicts with go.yaml.in/yaml/v3 used by yqlib dependency - Add missing perf.Track instrumentation to public functions in describe_component.go per coding guidelines - Stabilize provenance value hashing by using json.Marshal for deterministic serialization instead of fmt.Sprintf Fixes the CI build failure caused by YAML package type mismatch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: reduce cognitive complexity in provenance code * fix: resolve all golangci-lint errors and warnings - Extract constants for magic numbers and string literals - Refactor all functions with >5 params to use config structs - Fix getSymbolColor() to return different colors per symbol - Split tree_renderer.go into multiple files (under 500 lines) - Pass heavy params by pointer to avoid memory copying - Extract helper functions to reduce cyclomatic complexity - Fix all comment formatting (godot linter) - Fix nilerr issues - Fix nestif complexity in test by inverting conditions All golangci-lint issues resolved for PR #1584 * fix: resolve provenance tracking bug and linting issues - Fixed bug where __import__: and __import_meta__: provenance entries weren't being recorded for direct imports - Resolved race condition in parallel stack processing that caused wrong merge contexts to be stored - Added per-stack merge context storage to replace single global variable - Fixed comment formatting to end with periods (godot linter) - Resolved code duplication in describe_affected_test.go - Added "imports" to allowed fields in test - Provenance tracking is critical for showing users where configuration values come from - The bug prevented proper tracking of imports defined directly in stack files - Race conditions in parallel processing caused provenance to be attributed to wrong stack files - Pre-commit hooks enforce code quality standards 1. Added merge context initialization in ProcessYAMLConfigFileWithContext when nil 2. Created mergeContexts map to store contexts per stack file instead of single global 3. Added SetMergeContextForStack() and GetMergeContextForStack() functions 4. Updated ProcessYAMLConfigFiles to store each stack's context separately 5. Modified ExecuteDescribeComponentWithContext to retrieve stack-specific context 6. Maintained backward compatibility by keeping deprecated functions 7. Fixed all godot linter warnings in cmd files - TestDescribeComponent_DirectImportProvenance passes - TestDescribeComponent_NestedImportProvenance passes - TestDescribeComponentWithProvenance passes - All describe component tests pass - All linting checks pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: address CodeRabbit review comments - **pkg/provenance/data_transform.go**: Fix nil context handling to preserve sections when provenance tracking is disabled - Initialize `hasProvenance := ctx == nil` to keep all keys when no provenance tracking - Prevents data loss when provenance is disabled - **internal/exec/describe_component.go**: Fix TTY detection - Check `os.Stderr.Fd()` instead of `os.Stdout.Fd()` (correct stream) - Require both empty file and TTY to prevent spam to pipes/redirects - **internal/exec/describe_component.go**: Add safe type assertion for imports - Use safe `if impStr, ok := imp.(string)` instead of panic-prone `imp.(string)` - Skip non-string values gracefully - **pkg/provenance/text_utils.go**: Fix rune-safe text wrapping - Convert to `[]rune` before slicing to preserve multi-byte UTF-8 characters - Prevents corruption of ANSI sequences and Unicode text - **pkg/provenance/tree_renderer.go**: Handle highlight failures gracefully - Fall back to plain YAML on highlight error instead of propagating error - Ensures users always see YAML output - **docs/prd/import-provenance.md**: Add language identifiers to fenced code blocks - Change ` ``` ` to ` ```text ` for legend and architecture diagram - Fixes markdownlint warnings All fixes verified with: - Build: ✅ Passes - Linting: ✅ 0 issues - Tests: ✅ All pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: remove incomplete RenderInline() and consolidate to production implementation - **Removed `RenderInline()` function** - Incomplete placeholder that only returned plain YAML - **Updated tests** to use `RenderInlineProvenanceWithStackFile()` production implementation - **Renamed files** for clarity: - `inline.go` → `provenance_helpers.go` (now only contains helper utilities) - `inline_test.go` → `provenance_helpers_test.go` - **Kept `FormatProvenanceComment()`** - Useful helper for formatting provenance entries CodeRabbit correctly identified that `RenderInline()` was incomplete (just returned YAML without provenance comments). The PR already has a complete implementation in `RenderInlineProvenanceWithStackFile()` that: - Adds provenance legend - Parses YAML structure and builds path mapping - Looks up provenance for each key - Adds inline comments with file:line:column - Used in production code (describe_component.go:135) `RenderInline()` was only used in 2 unit tests and created confusion. Consolidating to a single implementation eliminates duplication and makes the codebase clearer. - ✅ Build passes - ✅ All tests pass (updated to use production function) - ✅ Linting passes (0 issues) - ✅ Single source of truth for inline provenance rendering Addresses CodeRabbit review: #1584 (comment) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * perf: add performance tracking to FormatProvenanceComment - Added `perf` import to `pkg/provenance/provenance_helpers.go` - Added `defer perf.Track(nil, "provenance.FormatProvenanceComment")()` to track performance All exported functions should have performance tracking for observability and profiling. This was missing from `FormatProvenanceComment()`. - ✅ Build passes - ✅ Tests pass - ✅ Linting passes (0 issues) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: remove unnecessary perf tracking from unused test helper - Removed `perf.Track()` call from `FormatProvenanceComment()` - Removed unused `perf` import `FormatProvenanceComment()` is only used in test code, not in production. Adding performance tracking to a test-only helper function provides no value and was causing CI failures on macOS and Windows platforms. - Commit `2374f64de` added perf tracking and caused CI failures - Linux: ✅ PASS - macOS: ❌ FAIL - Windows: ❌ FAIL The function is unused in production code - only called in its own test. Performance tracking on an unused helper is unnecessary and introduced platform-specific failures. - ✅ Build passes - ✅ Tests pass locally - ✅ Linting passes (0 issues) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: move formatProvenanceComment to test file - Moved `FormatProvenanceComment()` from `provenance_helpers.go` (production code) to `provenance_helpers_test.go` (test code) - Made function unexported (`formatProvenanceComment`) since it's test-only - Deleted `provenance_helpers.go` as it's now empty The function was only used in tests, not in production code. Having an exported function in production files that's only used in tests is misleading and was the reason CodeRabbit flagged it as an incomplete implementation. **Original Context:** - `FormatProvenanceComment()` was originally part of the incomplete `RenderInline()` implementation - When we removed `RenderInline()` (commit 99f4bb9), we kept this helper thinking it might be useful - But it's never actually used in production - only in its own test **Proper Organization:** - Test-only helpers belong in `_test.go` files - Should be unexported (lowercase) to indicate internal test usage - Keeps production code clean and focused - ✅ Build passes - ✅ All tests pass - ✅ Linting passes (0 issues) - ✅ No production code affected 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: ensure deterministic provenance tracking by sorting map keys Provenance tracking was non-deterministic due to Go's random map iteration order. This caused test failures on macOS CI where `TestDescribeComponentWithProvenance` expected to find specific paths like `vars.enabled` or `vars.name`, but map iteration could skip them depending on iteration order. Changes: - Sort map keys before iterating in `recordMapProvenance()` - Ensures consistent provenance path recording across all platforms - Fixes flaky test failures on macOS CI 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: address CodeRabbit review comments on provenance tracking CodeRabbit identified three issues with provenance implementation and tests: 1. **ValueHash not populated**: ProvenanceEntry.ValueHash field was empty - Added `value` parameter to provenanceEntryParams - Updated recordProvenanceEntry to compute and set ValueHash using hashValue() - Pass actual values through all recordProvenanceEntry calls - Changed params to pointer to satisfy gocritic hugeParam linter 2. **Missing negative assertions**: Test didn't verify provenance absence - Added assertions to ensure no provenance comments when ctx is nil - Test now checks for absence of "# from:" and "# Provenance Legend:" 3. **Weak provenance format assertions**: Test only checked for presence - Added exact format checks for provenance comments ("# ● [0] config.yaml:10") - Added legend symbol verification (●, ○, ∴) - Fixed bug where legend was always shown even when ctx is nil These changes ensure provenance entries have complete metadata for change detection and that tests properly verify both positive and negative cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: improve provenance display with parent stack header and depth 1 symbol Changes the provenance rendering to be more intuitive: 1. **Added parent stack header**: Shows which stack file is being described - Displays "# Stack: {filename}" before the YAML output - Helps users understand the context 2. **Fixed symbol logic**: Changed from depth 0 to depth 1 for parent stack - ● [1] now shows values defined in the parent stack - ○ [2+] shows values inherited from imports - Depth 0 never occurred in practice (parent stack is always depth 1) 3. **Updated legend text**: Reflects new depth numbering - "● [1] Defined in parent stack" - "○ [N] Inherited/imported (N=2+ levels deep)" 4. **Adjusted color coding**: Makes depths 1-2 green - Depth 1-2: green (parent + first import) - Depth 3: yellow (getting far) - Depth 4+: red (very far) 5. **Updated all tests**: Changed test expectations from depth 0 to depth 1 6. **Refactored legend rendering**: Extracted into helper function to reduce cyclomatic complexity This makes provenance output more intuitive and useful for understanding where configuration values come from. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * test: add comprehensive provenance output validation tests Add tests to validate complete provenance rendering output: - TestProvenanceOutputFormat: validates legend, stack header, symbols, and depth formatting for all depth levels - TestProvenanceComputedType: validates computed values show ∴ symbol regardless of depth 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: respect indent configuration when rendering YAML with provenance When rendering YAML with provenance comments, now respects the indent configuration from atmosConfig.Settings.Terminal.TabWidth instead of always using the default indent of 2. This ensures folded YAML scalars (like atmos_component_description with >-) are indented correctly according to the user's configuration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * test: add tests to validate YAML indent configuration for folded scalars Add comprehensive tests to verify that YAML folded scalar indentation respects the tab_width configuration: - TestYAMLIndentRespected: Validates indent=2 (default) produces 6 spaces - TestYAMLIndentWithIndent4: Validates indent=4 produces 12 spaces These tests ensure that the fix in tree_renderer.go correctly respects the TabWidth configuration when rendering YAML with provenance. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: disable syntax highlighting for provenance to fix folded scalar indentation The Chroma syntax highlighter was adding extra indentation (6 spaces) to YAML folded scalar continuation lines, causing them to display with 12 spaces instead of the correct 6 spaces. Root cause: When highlighting YAML with folded scalars (>-), the Chroma lexer/formatter adds duplicate indentation to continuation lines: - Original: " Vpc-Flow-Logs-Bucket..." (6 spaces) - After highlighting: " \x1b[0m Vpc-Flow..." (6 + 6 = 12 spaces) Solution: Disable syntax highlighting for provenance output. The YAML structure is already clear with provenance comments, and this avoids the indentation duplication issue. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: preserve syntax highlighting while fixing folded scalar indentation The Chroma syntax highlighter was duplicating indentation for YAML folded scalar continuation lines by inserting ANSI color codes within whitespace. This resulted in visual duplication (e.g., 6 spaces + color codes + 6 more spaces). Added post-processing that: - Compares original and highlighted YAML line-by-line - Counts leading spaces in both (handling ANSI codes in highlighted version) - Strips duplicate spaces when highlighted has >2x original indentation - Preserves all ANSI color codes for syntax highlighting Extracted helper functions to maintain code quality: - countLeadingSpaces() - counts spaces in plain strings - countLeadingSpacesWithANSI() - counts spaces while skipping ANSI codes - stripLeadingSpaces() - removes N spaces while preserving ANSI codes Also fixed unrelated linting issues: - Added static error ErrCommandNil for templater.go - Removed unused processHelp function and help.go file - Removed unused workflowMarkdown variable 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * chore: upgrade Chroma from v0.10.0 to v2.20.0 Upgraded to Chroma v2.20.0 which includes the fix for YAML multiline whitespace issues (fixed in v2.15.0 via PR #993). Changes: - Updated all Chroma imports from v0 to v2 - Updated go.mod dependencies - Removed old v0.10.0 dependency The v2.15.0 release includes "Don't output extra whitespace in YAML multiline" fix by @Gusted which should resolve the folded scalar indentation duplication issue. Kept the fixHighlightedIndentation() workaround as a safety net in case the upstream fix doesn't cover all edge cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * cleanup: remove workaround for Chroma YAML indentation bug Removed the fixHighlightedIndentation() workaround and helper functions since the Chroma v2.20.0 upgrade fixed the YAML multiline indentation issue. The upstream fix in Chroma v2.15.0 (PR #993) properly handles YAML folded scalars without adding duplicate whitespace, making our post-processing workaround unnecessary. Kept the stderr terminal detection enhancement as it's still needed to ensure syntax highlighting works when provenance output goes to stderr instead of stdout. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: send provenance output to stdout for piping Changed provenance output from stderr to stdout to enable piping: atmos describe component vpc --stack dev --provenance | grep pattern Changes: - Provenance output now goes to stdout (fmt.Print) - Simplified output logic: write to file if specified, else stdout - Removed unused golang.org/x/term import - Refactored to reduce complexity (extracted renderProvenance method) - Added type assertion with static error for provenance rendering This aligns with Unix philosophy and Atmos design: data goes to stdout, UI messages go to stderr. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Address CodeRabbit review comments for provenance feature - Fix comment punctuation in tree_renderer.go (add missing period) - Add performance tracking to buildFileTree function - Fix null value handling in merge provenance recording - Add comprehensive error condition tests - Enhance test assertions for provenance rendering - Ensures compliance with golangci-lint godot rules (all comments must end with periods) - Adds performance tracking to critical functions per coding guidelines - Fixes bug where nil values were not tracked in provenance (now treats nil as a scalar) - Improves test coverage with error conditions: nil atmosConfig, malformed entries, null values, nested structures, and different provenance types - Makes test assertions more specific and thorough, verifying exact comment formats and legend text - Addresses 6 unresolved CodeRabbit review comments on PR #1584 - Follows project guidelines in CLAUDE.md for comments, perf tracking, and test quality 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Fix data race in ProcessYAMLConfigFiles goroutines - Add mutex protection for errorResult variable accessed by multiple goroutines - CodeRabbit identified potential data race where multiple goroutines write to errorResult without synchronization - Lines 173, 205, and 222 all write to errorResult from different goroutines - Without synchronization, this could lead to race conditions and undefined behavior - Add errorLock sync.Mutex to protect errorResult writes - Wrap all errorResult assignments with errorLock.Lock()/Unlock() - Maintains existing processYAMLConfigFilesLock for map/slice access - Addresses CodeRabbit review comment on PR #1584 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Fix godot linter issue in merge_with_provenance.go parameter comments - Add periods to parameter comment lines that were ending with colons - golangci-lint godot rule requires all comments to end with periods - Parameter descriptions like "atmosConfig: ..." should end with "flag." not "flag" - Addresses linting issue identified in PR #1584 review 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * updates * updates * updates * updates * updates * updates --------- Co-authored-by: Claude <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: aknysh <[email protected]>
|
These changes were released in v1.194.0. |
what
--provenanceflag toatmos describe componentcommandpkg/ui/provenancepackage for rendering configuration provenancewhy
sourcesdata structure was already tracked but not displayed in a user-friendly wayreferences
ConfigSourcesinfrastructure from AtmosSummary by CodeRabbit
New Features
Documentation
UI
Tests