feat: YAML key delimiter for dot notation in stack files#2139
Open
feat: YAML key delimiter for dot notation in stack files#2139
Conversation
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
mergify
bot
added
conflict
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds opt-in YAML key-delimiter expansion for dotted keys, schema/config fields and Go types to control it, a new yaml/expand package with tests, integration into YAML parsing, an experimental gating check in the root command, and documentation + a blog post. Changes
Sequence Diagram(s)sequenceDiagram
participant Root as Root Command
participant ExpCheck as Experimental Gate
participant YAMLParse as YAML Parser
participant Expand as Key Delimiter Expander
participant Config as Config Object
Root->>ExpCheck: checkExperimentalSettings(atmosConfig)
ExpCheck->>ExpCheck: read settings.yaml.key_delimiter and mode
ExpCheck-->>Root: allow / warn / error / exit
Root->>YAMLParse: parseAndCacheYAML(file)
YAMLParse->>YAMLParse: extract provenance positions
YAMLParse->>Expand: KeyDelimiters(node, atmosConfig.Settings.YAML.KeyDelimiter)
Expand->>Expand: traverse AST, split keys, build nested maps, merge (last-wins)
Expand-->>YAMLParse: return expanded YAML AST
YAMLParse->>YAMLParse: process custom tags, decode to Config
YAMLParse-->>Config: parsed configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
cmd/root.go (1)
905-953: Consider unifying command/settings experimental mode handling.Line 928+ duplicates the same mode-switch behavior already used for command gating. A shared helper would reduce drift risk when modes, hints, or error semantics evolve.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/root.go` around lines 905 - 953, The checkExperimentalSettings function duplicates the experimental-mode switch logic used for command gating; extract that switch into a shared helper (e.g., applyExperimentalMode or handleExperimentalFeature) that accepts the feature string, the experimentalMode value and contextual metadata (like context type "setting" vs "command" and hint text) and use it from both checkExperimentalSettings and the command gating code path; update checkExperimentalSettings to build the feature identifier(s) (e.g., "settings.yaml.key_delimiter"), determine experimentalMode defaulting to "warn", and call the new helper so the silence/disable/warn/error behavior, error types (ErrExperimentalDisabled/ErrExperimentalRequiresIn), UI call ui.Experimental, and errUtils.CheckErrorPrintAndExit invocations are centralized.website/docs/reference/yaml-reference.mdx (1)
253-258: Helpful tooling warning.Good to set expectations about IDE/linter compatibility.
Minor style nit: "YAML language servers" is slightly redundant since YAML stands for "Yet Another Markup Language." Consider "YAML servers" or "language servers" instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/reference/yaml-reference.mdx` around lines 253 - 258, Summary: Minor style nit—avoid redundancy in the phrase "YAML language servers." Edit the warning block titled "External Tooling" and replace the phrase "YAML language servers" with a non-redundant alternative such as "language servers" (or "YAML servers") so the sentence reads clearly; keep the rest of the warning text unchanged and preserve the example mentioning `metadata.component`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/utils/yaml_utils.go`:
- Around line 281-285: The YAML parse cache must include the delimiter so parse
results reflect delimiter-dependent expansion; update the cache key generation
(the code that caches the parsed tree for parsedNode) to incorporate
atmosConfig.Settings.YAML.KeyDelimiter (or an explicit delimiter value) along
with the file and content fingerprint, and ensure
expand.KeyDelimiters(&parsedNode, atmosConfig.Settings.YAML.KeyDelimiter) still
runs after parsing; this makes cache lookups consider the delimiter and prevents
returning a tree expanded with a different key_delimiter.
In `@website/src/data/roadmap.js`:
- Line 217: The roadmap entry object with label 'YAML key delimiter for dot
notation' is missing the required pr metadata; add a pr property (e.g., pr:
'<PR_NUMBER>') to that shipped milestone object so it includes status, changelog
and pr fields for CI traceability, and then update the related initiative
progress percentage according to the project guidelines so the roadmap CI check
passes.
---
Nitpick comments:
In `@cmd/root.go`:
- Around line 905-953: The checkExperimentalSettings function duplicates the
experimental-mode switch logic used for command gating; extract that switch into
a shared helper (e.g., applyExperimentalMode or handleExperimentalFeature) that
accepts the feature string, the experimentalMode value and contextual metadata
(like context type "setting" vs "command" and hint text) and use it from both
checkExperimentalSettings and the command gating code path; update
checkExperimentalSettings to build the feature identifier(s) (e.g.,
"settings.yaml.key_delimiter"), determine experimentalMode defaulting to "warn",
and call the new helper so the silence/disable/warn/error behavior, error types
(ErrExperimentalDisabled/ErrExperimentalRequiresIn), UI call ui.Experimental,
and errUtils.CheckErrorPrintAndExit invocations are centralized.
In `@website/docs/reference/yaml-reference.mdx`:
- Around line 253-258: Summary: Minor style nit—avoid redundancy in the phrase
"YAML language servers." Edit the warning block titled "External Tooling" and
replace the phrase "YAML language servers" with a non-redundant alternative such
as "language servers" (or "YAML servers") so the sentence reads clearly; keep
the rest of the warning text unchanged and preserve the example mentioning
`metadata.component`.
ℹ️ Review info
Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f105c6a9-f55d-4d89-81f2-ab8ccf8986c8
📒 Files selected for processing (11)
cmd/root.gopkg/datafetcher/schema/atmos/manifest/1.0.jsonpkg/datafetcher/schema/config/global/1.0.jsonpkg/schema/schema.gopkg/utils/yaml_utils.gopkg/yaml/expand/expand.gopkg/yaml/expand/expand_test.gowebsite/blog/2026-03-03-yaml-key-delimiter.mdxwebsite/docs/cli/configuration/settings/settings.mdxwebsite/docs/reference/yaml-reference.mdxwebsite/src/data/roadmap.js
Comment on lines
+281
to
+285
| // Expand delimited keys (e.g., "a.b: v" -> "a: {b: v}") if configured. | ||
| // This runs before custom tag processing so expanded keys are available for tag resolution. | ||
| if atmosConfig != nil && atmosConfig.Settings.YAML.KeyDelimiter != "" { | ||
| expand.KeyDelimiters(&parsedNode, atmosConfig.Settings.YAML.KeyDelimiter) | ||
| } |
Contributor
There was a problem hiding this comment.
Cache key must include delimiter-dependent parse behavior.
Line 283 makes parse output depend on settings.yaml.key_delimiter, but cached entries are still keyed by file+content only. This can return incorrect cached trees when delimiter differs across loads in the same process.
Suggested direction
-func generateParsedYAMLCacheKey(file string, content string) string {
+func generateParsedYAMLCacheKey(file string, content string, keyDelimiter string) string {
@@
- return file + ":" + contentHash
+ return file + ":" + contentHash + ":" + keyDelimiter
}-func getCachedParsedYAML(file string, content string) (*yaml.Node, PositionMap, bool) {
- cacheKey := generateParsedYAMLCacheKey(file, content)
+func getCachedParsedYAML(file string, content string, keyDelimiter string) (*yaml.Node, PositionMap, bool) {
+ cacheKey := generateParsedYAMLCacheKey(file, content, keyDelimiter)-func cacheParsedYAML(file string, content string, node *yaml.Node, positions PositionMap) {
- cacheKey := generateParsedYAMLCacheKey(file, content)
+func cacheParsedYAML(file string, content string, keyDelimiter string, node *yaml.Node, positions PositionMap) {
+ cacheKey := generateParsedYAMLCacheKey(file, content, keyDelimiter)+ keyDelimiter := ""
+ if atmosConfig != nil {
+ keyDelimiter = atmosConfig.Settings.YAML.KeyDelimiter
+ }
@@
- cacheParsedYAML(file, input, &parsedNode, positions)
+ cacheParsedYAML(file, input, keyDelimiter, &parsedNode, positions)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/utils/yaml_utils.go` around lines 281 - 285, The YAML parse cache must
include the delimiter so parse results reflect delimiter-dependent expansion;
update the cache key generation (the code that caches the parsed tree for
parsedNode) to incorporate atmosConfig.Settings.YAML.KeyDelimiter (or an
explicit delimiter value) along with the file and content fingerprint, and
ensure expand.KeyDelimiters(&parsedNode, atmosConfig.Settings.YAML.KeyDelimiter)
still runs after parsing; this makes cache lookups consider the delimiter and
prevents returning a tree expanded with a different key_delimiter.
| { label: 'Filename-based stack identity (zero-config)', status: 'shipped', quarter: 'q1-2026', pr: 1934, changelog: 'stack-name-identity', description: 'Stacks are identified by filename when no name, name_template, or name_pattern is configured. Newcomers can get started without any naming configuration.', benefits: 'New users can start immediately without configuring stack naming. Just create stack files and reference them by filename.' }, | ||
| { label: 'Command aliases for vendor and workflow list', status: 'shipped', quarter: 'q1-2026', changelog: 'vendor-workflow-list-aliases', description: 'Added `atmos vendor list` and `atmos workflow list` as aliases for their `atmos list` counterparts for intuitive command discovery.', benefits: 'Users can use either command form. Natural command structure regardless of preference.' }, | ||
| { label: 'Packer directory-based templates', status: 'shipped', quarter: 'q1-2026', pr: 1982, changelog: 'packer-directory-based-templates', description: 'Packer commands now default to directory mode, loading all *.pkr.hcl files from the component directory. Aligns with HashiCorp best practices for multi-file configurations.', benefits: 'Organize Packer configurations across multiple files without explicit template configuration. Just run atmos packer build and it works.' }, | ||
| { label: 'YAML key delimiter for dot notation', status: 'shipped', quarter: 'q1-2026', changelog: 'yaml-key-delimiter', docs: '/reference/yaml#dot-notation-for-nested-keys', description: 'Configurable key delimiter expands dotted YAML keys into nested maps in stack files. Quoted keys stay literal. Supports custom delimiters.', benefits: 'Write concise dot notation like metadata.component: vpc-base instead of deeply nested YAML. Opt-in and backwards compatible.', experimental: true }, |
Contributor
There was a problem hiding this comment.
Add pr metadata to the shipped milestone.
Line 217 adds a shipped milestone but omits the pr field, which breaks shipped-item traceability expectations for roadmap entries.
Suggested patch
- { label: 'YAML key delimiter for dot notation', status: 'shipped', quarter: 'q1-2026', changelog: 'yaml-key-delimiter', docs: '/reference/yaml#dot-notation-for-nested-keys', description: 'Configurable key delimiter expands dotted YAML keys into nested maps in stack files. Quoted keys stay literal. Supports custom delimiters.', benefits: 'Write concise dot notation like metadata.component: vpc-base instead of deeply nested YAML. Opt-in and backwards compatible.', experimental: true },
+ { label: 'YAML key delimiter for dot notation', status: 'shipped', quarter: 'q1-2026', pr: 2139, changelog: 'yaml-key-delimiter', docs: '/reference/yaml#dot-notation-for-nested-keys', description: 'Configurable key delimiter expands dotted YAML keys into nested maps in stack files. Quoted keys stay literal. Supports custom delimiters.', benefits: 'Write concise dot notation like metadata.component: vpc-base instead of deeply nested YAML. Opt-in and backwards compatible.', experimental: true },As per coding guidelines website/src/data/roadmap.js: “Roadmap Updates (CI Enforced): ... Add shipped milestones with status/changelog/pr fields and update initiative progress percentage.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { label: 'YAML key delimiter for dot notation', status: 'shipped', quarter: 'q1-2026', changelog: 'yaml-key-delimiter', docs: '/reference/yaml#dot-notation-for-nested-keys', description: 'Configurable key delimiter expands dotted YAML keys into nested maps in stack files. Quoted keys stay literal. Supports custom delimiters.', benefits: 'Write concise dot notation like metadata.component: vpc-base instead of deeply nested YAML. Opt-in and backwards compatible.', experimental: true }, | |
| { label: 'YAML key delimiter for dot notation', status: 'shipped', quarter: 'q1-2026', pr: 2139, changelog: 'yaml-key-delimiter', docs: '/reference/yaml#dot-notation-for-nested-keys', description: 'Configurable key delimiter expands dotted YAML keys into nested maps in stack files. Quoted keys stay literal. Supports custom delimiters.', benefits: 'Write concise dot notation like metadata.component: vpc-base instead of deeply nested YAML. Opt-in and backwards compatible.', experimental: true }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/src/data/roadmap.js` at line 217, The roadmap entry object with label
'YAML key delimiter for dot notation' is missing the required pr metadata; add a
pr property (e.g., pr: '<PR_NUMBER>') to that shipped milestone object so it
includes status, changelog and pr fields for CI traceability, and then update
the related initiative progress percentage according to the project guidelines
so the roadmap CI check passes.
Implement configurable key delimiter expansion that transforms dotted YAML keys into nested maps during parsing, bringing stack files in line with atmos.yaml (which already supports dotted keys via Viper). - Add `settings.yaml.key_delimiter` configuration option (opt-in, backwards compatible) - Implement Node-level YAML expansion respecting quoted key preservation - Unquoted keys containing the delimiter expand to nested maps - Quoted keys (single/double) stay literal, enabling escape mechanism - Support custom delimiters (not just `.`, could be `::` or any string) - Mark feature as experimental (gated by `settings.experimental` mode) - Add runtime warning via `ui.Experimental()` when setting is enabled - Add comprehensive unit tests (21 tests covering edge cases) Users want concise dot notation (`metadata.component: vpc-base`) instead of deeply nested YAML. Documentation claimed this worked in stack files but it didn't—Go's yaml.v3 treats dotted keys literally without expansion. This brings feature parity with atmos.yaml's native support via Viper's `deepSearch()` algorithm. Backwards compatible (disabled by default). Feature is marked experimental and respects `settings.experimental` mode (silence/warn/error/disable). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
osterman
force-pushed
the
osterman/yaml-dot-notation-test
branch
from
babe1be to
dda8bca
Compare
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/root.go (1)
905-953: Extract shared experimental-mode handling to avoid behavior drift.This mode switch duplicates the command-level experimental switch already in this file. Centralizing both paths into one helper will keep
silence/warn/error/disablesemantics in lockstep.♻️ Proposed refactor
+func handleExperimentalMode(contextKey, contextValue, mode string) { + if mode == "" { + mode = "warn" + } + switch mode { + case "silence": + return + case "disable": + errUtils.CheckErrorPrintAndExit( + errUtils.Build(errUtils.ErrExperimentalDisabled). + WithContext(contextKey, contextValue). + WithHint("Enable with settings.experimental: warn"). + Err(), + "", "", + ) + case "warn": + ui.Experimental(contextValue) + case "error": + ui.Experimental(contextValue) + errUtils.CheckErrorPrintAndExit( + errUtils.Build(errUtils.ErrExperimentalRequiresIn). + WithContext(contextKey, contextValue). + WithHint("Enable with settings.experimental: warn"). + Err(), + "", "", + ) + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/root.go` around lines 905 - 953, The experimental-mode switch in checkExperimentalSettings duplicates command-level logic; extract the switch into a single helper (e.g., applyExperimentalMode(feature string, mode string)) that encapsulates the cases "silence", "disable", "warn", "error" and uses ui.Experimental, errUtils.Build and errUtils.CheckErrorPrintAndExit in the same way as the current switch; then replace the inline switch in checkExperimentalSettings with a call to applyExperimentalMode(feature, experimentalMode) and update the command-level code to call the same helper so both paths share identical behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/datafetcher/schema/atmos/manifest/1.0.json`:
- Around line 617-627: The schema for the "yaml" object under settings is too
strict and will reject custom keys; change the "yaml" definition (the "yaml"
property in definitions.settings) to allow additional properties (remove or set
"additionalProperties" to true) while keeping the existing "key_delimiter"
property and its description unchanged so existing custom data in settings.yaml
remains compatible.
---
Nitpick comments:
In `@cmd/root.go`:
- Around line 905-953: The experimental-mode switch in checkExperimentalSettings
duplicates command-level logic; extract the switch into a single helper (e.g.,
applyExperimentalMode(feature string, mode string)) that encapsulates the cases
"silence", "disable", "warn", "error" and uses ui.Experimental, errUtils.Build
and errUtils.CheckErrorPrintAndExit in the same way as the current switch; then
replace the inline switch in checkExperimentalSettings with a call to
applyExperimentalMode(feature, experimentalMode) and update the command-level
code to call the same helper so both paths share identical behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ee0e865f-25c9-4e8b-b7d4-2182aa5d8055
📒 Files selected for processing (11)
cmd/root.gopkg/datafetcher/schema/atmos/manifest/1.0.jsonpkg/datafetcher/schema/config/global/1.0.jsonpkg/schema/schema.gopkg/utils/yaml_utils.gopkg/yaml/expand/expand.gopkg/yaml/expand/expand_test.gowebsite/blog/2026-03-03-yaml-key-delimiter.mdxwebsite/docs/cli/configuration/settings/settings.mdxwebsite/docs/reference/yaml-reference.mdxwebsite/src/data/roadmap.js
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/utils/yaml_utils.go
- pkg/yaml/expand/expand_test.go
- pkg/datafetcher/schema/config/global/1.0.json
- website/src/data/roadmap.js
Remove additionalProperties: false from the yaml object in settings schema definitions to avoid rejecting custom keys users may have under settings.yaml. The Go struct controls what is actually parsed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add yaml.key_delimiter field to describe config golden snapshots to match the new settings.yaml struct field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (68.37%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2139 +/- ##
==========================================
- Coverage 76.57% 76.57% -0.01%
==========================================
Files 832 833 +1
Lines 79524 79641 +117
==========================================
+ Hits 60898 60983 +85
- Misses 14830 14857 +27
- Partials 3796 3801 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
settings.yaml.key_delimiterthat expands dotted YAML keys into nested maps in stack files (e.g.,metadata.component: vpc-base→metadata: { component: vpc-base }).(e.g.,::)ui.Experimental()why
Documentation claimed dot notation worked in stack files but it didn't—Go's
yaml.v3treats dotted keys literally without expansion. Users want concise notation instead of deeply nested YAML structures. This brings feature parity withatmos.yaml's native support via Viper'sdeepSearch()algorithm. Backwards compatible (disabled by default) and opt-in.references
Extends experimental features system (like
atmos devcontainer,atmos list affected). Respectssettings.experimentalmode configuration (silence/warn/error/disable).Summary by CodeRabbit
New Features
Documentation