Replace regex-based interpolation with character scanner, add "did you mean" suggestions#4715
Draft
shreyas-goenka wants to merge 7 commits intomainfrom
Draft
Replace regex-based interpolation with character scanner, add "did you mean" suggestions#4715shreyas-goenka wants to merge 7 commits intomainfrom
shreyas-goenka wants to merge 7 commits intomainfrom
Conversation
Two proposals:
1. Replace regex-based ${...} parsing with a proper two-mode character scanner
2. Add "did you mean?" suggestions for invalid variable references
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Captures the codebase map, key files, invariants, and instructions needed for a new session to implement the two design proposals. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…"did you mean" suggestions
- Replace regex in dynvar/ref.go with a two-mode character scanner that
produces typed tokens (literal/ref), enabling proper error reporting
for malformed variable references like ${foo.bar-} or ${foo..bar}
- Add Levenshtein-based "did you mean" suggestions when a valid reference
fails to resolve (e.g., ${var.my_clster_id} -> did you mean ${var.my_cluster_id}?)
- Add $$ escape sequence for producing literal ${ in output
- Add standalone WarnMalformedReferences mutator in initialize phase
- Update token-based string interpolation in resolver
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
…context Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
|
Commit: 12949c5
16 interesting tests: 7 SKIP, 6 RECOVERED, 3 flaky
Top 12 slowest tests (at least 2 minutes):
|
Split path validation into per-segment traversal with a small key regex instead of one big regex for the entire path. This produces better error messages pointing to the exact invalid segment, e.g.: "invalid key "bar-"" instead of "invalid path" Also drops the unhelpful "at position N" from error messages since the YAML file location is already reported. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
denik
reviewed
Mar 12, 2026
|
|
||
| 1. **Silent failures** — `${foo.bar-}` silently treated as literal text with no warning. | ||
| 2. **No suggestions** — `${bundle.nme}` produces "reference does not exist" with no hint. | ||
| 3. **No escape mechanism** — no way to produce a literal `${` in output. |
Contributor
There was a problem hiding this comment.
- We cannot use all features we have in libs/structs/structpath such as key-value references tasks[task_key="x"].
| reference resolves first. The old regex matched only `${var.tail}` (the | ||
| innermost pair). The new parser preserves this: when scanning for `}` inside | ||
| a reference, if another `${` is encountered, the outer `${` is abandoned | ||
| (treated as literal) and scanning restarts from the inner `${`. |
Contributor
There was a problem hiding this comment.
Is that what we want? I think we've discussed that we want to reject these constructs.
| a reference, if another `${` is encountered, the outer `${` is abandoned | ||
| (treated as literal) and scanning restarts from the inner `${`. | ||
|
|
||
| ### `$$` escape sequence |
Contributor
There was a problem hiding this comment.
We've discussed in this in core eng meeting and there were a number of concerns with this. Notably, in bash it's current process.
I think least controversial option was: \$ -> $ \\ -> \
| @@ -0,0 +1,207 @@ | |||
| package interpolation | |||
Contributor
There was a problem hiding this comment.
It should not be in dyn package, can be top level in libs. We might remove libs/dyn, but this will stay.
| } | ||
|
|
||
| func TestParseEscapeDollar(t *testing.T) { | ||
| t.Run("double_dollar", func(t *testing.T) { |
Contributor
There was a problem hiding this comment.
Why use inner test here if there is no common setup?
Why not merge these into table test above?
| // SuggestPath walks the reference path against root, correcting mistyped key | ||
| // components via fuzzy matching. Returns the corrected path string, or "" if | ||
| // the path cannot be corrected. | ||
| func SuggestPath(root dyn.Value, refPath dyn.Path) string { |
Contributor
There was a problem hiding this comment.
Can we make it independent of dyn?
Can we split suggestion into separate PR (both features are pretty big).
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
3 participants
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.
Summary
dynvar/ref.gowith a two-mode character scanner (libs/dyn/dynvar/interpolation/parse.go) that produces typed tokens (literal/reference), enabling proper error reporting for malformed references like${foo.bar-}or${foo..bar}${var.my_clster_id}→did you mean ${var.my_cluster_id}?)$$escape sequence for producing literal${in output (following HCL2 precedent)WarnMalformedReferencesmutator in the initialize phase that walks the config tree once to emit warnings for malformed variable referencesstrings.Replace-based interpolation with token-based concatenation in the resolverDesign doc:
design/interpolation-parser.mdTest plan
go test ./libs/dyn/dynvar/interpolation/...— parser unit testsgo test ./libs/dyn/dynvar/...— ref, resolve, suggest testsgo test ./bundle/config/mutator/...— mutator testsgo test ./acceptance -run TestAccept/bundle/variables/malformed_referencego test ./acceptance -run TestAccept/bundle/variables/did_you_mean🤖 Generated with Claude Code