feat: handle flow sequences gracefully in sync, insert, append, extend_list#32
Merged
nathanjmcdougall merged 11 commits intoMay 20, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves yamltrip.Document list-mutation ergonomics by making sync(), insert(), append(), and extend_list() handle YAML flow sequences (e.g. items: [a, b], items: []) without raising PatchError, generally by falling back to a whole-value replace() that converts the node to a block-style representation. It also introduces a NodeTypeError exception to better distinguish “wrong node type for operation” from other patch failures.
Changes:
- Add flow-sequence fallbacks for
sync(),insert(),append(), andextend_list()(replace whole list when patching can’t mutate a flow sequence). - Introduce
NodeTypeError(PatchError, TypeError)and use it for clearer type-mismatch failures (e.g.remove_from_list()on non-lists). - Add/expand test coverage for flow sequence behaviors and
NodeTypeErrorinheritance/usage.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/yamltrip/document.py |
Adds flow-sequence pre-conversion for sync() and fallback behaviors for list mutations; raises NodeTypeError for type mismatches. |
src/yamltrip/errors.py |
Defines the new NodeTypeError exception type. |
src/yamltrip/__init__.py |
Exports NodeTypeError as part of the public API. |
tests/test_sync.py |
Adds flow-sequence-focused sync() tests, including nested mapping cases and sibling-comment preservation. |
tests/test_document.py |
Adds flow-sequence tests for append()/extend_list()/insert() and adds NodeTypeError behavioral tests. |
tests/test_errors.py |
Verifies NodeTypeError inherits from both PatchError and TypeError. |
doc/specs/2026-05-20-flow-sequence-fallback-design.md |
New design spec documenting the flow-sequence fallback approach. |
doc/specs/2026-05-20-node-type-error-design.md |
New design spec documenting the NodeTypeError rationale and hierarchy. |
uv.lock |
Updates the editable yamltrip package version reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Closes #31
Summary
Previously,
sync(),insert(),append(), andextend_list()raisedPatchErrorwhen operating on flow sequences (e.g.repos: []oritems: [a, b, c]). This forced callers to wrap calls in try/except and fall back toupsert().Now these methods detect the flow sequence error and automatically fall back to a full
replaceoperation, which converts the flow sequence to a block sequence. This is always correct because:[]) have no formatting to preserveChanges
Flow sequence fallback (Category A)
sync()— catchesPatchError("expected BlockSequence")and falls back toOp.replace(value)insert()— catches the error, doeslist.insert()in Python, replaces the whole listappend()— catchesPatchError("flow sequence"), appends in Python, replacesextend_list()— same pattern as appendNodeTypeError (Category B)
NodeTypeError(PatchError, TypeError)for when operations are called on the wrong node typeremove_from_list()now raisesNodeTypeErrorinstead of rawPatchErrorwhen the target isn't a listexcept PatchErrorhandlers still catch itTest coverage
sync()on flow sequencesinsert()on flow sequencesappend()/extend_list()on flow sequencesNodeTypeErrorhierarchy