feat: support short-circuiting append operations when there are multiple appends, not just 1 item#153
Conversation
…ray item, we do not need to delete and recreate the array
There was a problem hiding this comment.
Pull request overview
Adds support in the overlay comparison algorithm to emit a single “append” update action when a YAML sequence has multiple new trailing elements (not just one), and validates the behavior with a new unit test.
Changes:
- Generalize the sequence tail-append short-circuit from
+1element to+Nelements inCompare. - Add a regression test ensuring multiple appended items produce a single append action and round-trip correctly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| overlay/compare.go | Extends the sequence append short-circuit to handle multiple trailing additions by emitting one update action containing the appended tail. |
| overlay/compare_test.go | Adds a test covering multi-element sequence append and verifies the overlay applies cleanly to reproduce the target YAML. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if len(y2.Content) > len(y1.Content) && | ||
| yamlEquals(y2.Content[:len(y1.Content)], y1.Content) { | ||
| return []Action{{ | ||
| Target: path.ToJSONPath(), | ||
| Update: yaml.Node{ | ||
| Kind: y1.Kind, | ||
| Content: []*yaml.Node{y2.Content[len(y1.Content)]}, | ||
| Content: y2.Content[len(y1.Content):], | ||
| }, |
There was a problem hiding this comment.
The new short-circuit path now calls yamlEquals(...) for any case where y2 is longer than y1, even when the sequence changes are not a pure tail-append. Since yamlEquals re-encodes each element to YAML strings, this can add noticeable overhead on large arrays/specs. Consider adding a cheaper structural equality check (or an early-exit comparison on Kind/Tag/Value) before falling back to the encoder-based comparison, so non-append cases can skip the expensive work quickly.
📊 Test Coverage ReportCurrent Coverage: Coverage Change: ✅ No change Coverage by Package
📋 Detailed Coverage by Function (click to expand)
Generated by GitHub Actions |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
No description provided.