fix(overlay): keep folded block scalars stable across repeated applies - #2116
fix(overlay): keep folded block scalars stable across repeated applies#2116AshGodfrey wants to merge 1 commit into
Conversation
gopkg.in/yaml.v3 emits an extra line break before a more-indented line inside a folded (>-) scalar. Applying an overlay is a decode/encode round trip, so each apply grew such a scalar by one blank line, and the break became part of the decoded string rather than cosmetic whitespace. A source with many overlays accumulated a blank line per overlay in descriptions no overlay targeted, and those reached generated SDK and provider doc strings. Re-style affected folded scalars as literal blocks before rendering. Literal blocks reproduce their value verbatim and round trip unchanged. The decoded value is identical; only the on-disk representation changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/overlay/overlay.go">
<violation number="1" location="pkg/overlay/overlay.go:123">
P2: When a folded scalar starts with a blank line and also contains a more-indented line, this call changes its decoded value on the first apply. Preserve folded style for leading-newline values or update the stabilizer to emit a literal block that retains the initial blank line.</violation>
</file>
<file name="pkg/overlay/foldedscalar.go">
<violation number="1" location="pkg/overlay/foldedscalar.go:29">
P2: Explicitly tagged folded scalars bypass stabilization because `Style` is a bitmask and includes `TaggedStyle`. Test the folded bit and replace only that bit while preserving the tag, so tagged descriptions cannot keep gaining blank lines.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| } | ||
| } | ||
|
|
||
| stabilizeFoldedScalars(document) |
There was a problem hiding this comment.
P2: When a folded scalar starts with a blank line and also contains a more-indented line, this call changes its decoded value on the first apply. Preserve folded style for leading-newline values or update the stabilizer to emit a literal block that retains the initial blank line.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/overlay/overlay.go, line 123:
<comment>When a folded scalar starts with a blank line and also contains a more-indented line, this call changes its decoded value on the first apply. Preserve folded style for leading-newline values or update the stabilizer to emit a literal block that retains the initial blank line.</comment>
<file context>
@@ -120,6 +120,8 @@ func apply(document *yaml.Node, o *overlay.Overlay, sourceLocation string, yamlI
}
}
+ stabilizeFoldedScalars(document)
+
bytes, err := schemas.RenderDocument(document, sourceLocation, yamlIn, yamlOut)
</file context>
| if node.Kind == yaml.ScalarNode && node.Style == yaml.FoldedStyle && hasMoreIndentedLine(node.Value) { | ||
| node.Style = yaml.LiteralStyle |
There was a problem hiding this comment.
P2: Explicitly tagged folded scalars bypass stabilization because Style is a bitmask and includes TaggedStyle. Test the folded bit and replace only that bit while preserving the tag, so tagged descriptions cannot keep gaining blank lines.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/overlay/foldedscalar.go, line 29:
<comment>Explicitly tagged folded scalars bypass stabilization because `Style` is a bitmask and includes `TaggedStyle`. Test the folded bit and replace only that bit while preserving the tag, so tagged descriptions cannot keep gaining blank lines.</comment>
<file context>
@@ -0,0 +1,36 @@
+ return
+ }
+
+ if node.Kind == yaml.ScalarNode && node.Style == yaml.FoldedStyle && hasMoreIndentedLine(node.Value) {
+ node.Style = yaml.LiteralStyle
+ }
</file context>
| if node.Kind == yaml.ScalarNode && node.Style == yaml.FoldedStyle && hasMoreIndentedLine(node.Value) { | |
| node.Style = yaml.LiteralStyle | |
| if node.Kind == yaml.ScalarNode && node.Style&yaml.FoldedStyle != 0 && hasMoreIndentedLine(node.Value) { | |
| node.Style = (node.Style &^ yaml.FoldedStyle) | yaml.LiteralStyle |
|
There are a few places we use the overlay package directly. Could we apply this to that package instead? Feels like this would be better to upstream |
|
Closing in favor of speakeasy-api/openapi#244, which applies the same fix in the |
Why
gopkg.in/yaml.v3emits an extra line break immediately before a more-indented line inside a folded (>-) scalar. Applying an overlay is a decode/mutate/encode round trip, so every apply grew such a scalar by one blank line. The break lands inside the scalar, so it becomes part of the decoded string rather than cosmetic whitespace.A source configured with many overlays accumulated roughly one blank line per overlay in descriptions that no overlay targeted, and those inflated descriptions reached generated SDK and provider doc strings.
Minimal reproduction, no overlays needed:
Folded scalars without a more-indented line, and literal (
|-) blocks, are already stable.What changed
stabilizeFoldedScalarsre-styles folded scalars containing a more-indented line as literal blocks, called inapply()beforeRenderDocument. Literal blocks reproduce their value verbatim and round trip unchanged.This removes the trigger rather than patching the emitter, so it needs no fork or
replacedirective. The decoded value is unchanged — the node already holds the folded value — so only the on-disk representation differs.apply()is the shared path forspeakeasy run,speakeasy overlay apply, CI document generation, and registry application, so all four are covered.Testing
go test ./pkg/overlay/...passes, including the pre-existing expected-output fixtures.TestFoldedScalarGrowsWithoutStabilizerasserts the unpatched emitter still misbehaves, so it fails ifyaml.v3is ever fixed or bumped and the workaround can be removed.Note for reviewers
Affected descriptions change representation from
>-to|on their first regeneration after this lands. That is a one-time diff; output is stable afterwards.Two call sites still invoke
ApplyTodirectly and bypass this path:internal/suggest/suggest.goandpkg/transform/filterOperations.go. Neither is on the regeneration path, so they are left out of this change.🤖 Generated with Claude Code
Summary by cubic
Stabilizes YAML folded (
>-) scalars across repeated overlay applies. Previously, each apply added a blank line before any more‑indented line due togopkg.in/yaml.v3; now we re-style those scalars to literal (|) before render so values round-trip unchanged. Decoded strings stay the same; only on-disk representation changes.apply()viastabilizeFoldedScalars, sospeakeasy run,speakeasy overlay apply, CI doc generation, and registry application all benefit.>-to|on first regeneration; subsequent outputs are stable.internal/suggest/suggest.go,pkg/transform/filterOperations.go) still callApplyTodirectly and are not on the regeneration path. Tests assert currentyaml.v3behavior and stability to flag removal if the emitter is fixed.Written for commit 1a2b3b1. Summary will update on new commits.