Skip to content

Commit 302d1b7

Browse files
ryan-williamsclaude
andcommitted
Fix insert_key_between bug with nested structures
Fixes bug where inserting after a key with deeply nested values would incorrectly move the last item from the nested structure into the new key's value. **The Problem:** _find_key_byte_range() was only looking one level deep when finding the end of nested structures. For example, with: ```yaml strategy: matrix: python-version: ["3.11"] jax-version: ["0.5.2", "0.6.2"] ``` It would find the end at `python-version` line instead of `jax-version` line, causing `jax-version` to be incorrectly captured and moved into the next inserted key. **The Fix:** Added recursive `find_last_line()` helper that traverses nested CommentedMaps and CommentedSeqs to find the actual last line of deeply nested structures. **Test:** Added regression test that verifies: - strategy.matrix keeps both python-version and jax-version - defaults.run only has working-directory - jax-version doesn't leak into defaults.run All 38 tests passing ✅ This fixes the workspace migration issue where strategy.matrix values were being corrupted when adding defaults sections. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 0904800 commit 302d1b7

2 files changed

Lines changed: 81 additions & 13 deletions

File tree

src/yaya/document.py

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -372,24 +372,45 @@ def _find_key_byte_range(self, parent: CommentedMap, key: str) -> tuple[int, int
372372
val_start, val_end = find_scalar_value_range(self.original_bytes, val_line, val_col)
373373

374374
# For non-scalar values (maps, sequences), we need to find the actual end
375+
# by recursively finding the last item in nested structures
375376
value = parent[key]
376377
if isinstance(value, (CommentedMap, CommentedSeq)):
377-
# Find the end by looking for the last item's position
378378
val_end_line = val_line
379-
if hasattr(value, 'lc') and hasattr(value.lc, 'data'):
380-
# Get the last item's position
381-
if isinstance(value, CommentedMap) and value:
382-
last_key = list(value.keys())[-1]
383-
if last_key in value.lc.data:
384-
last_info = value.lc.data[last_key]
379+
380+
# Recursively find the deepest last item
381+
def find_last_line(obj):
382+
"""Recursively find the line number of the last item in nested structures."""
383+
if isinstance(obj, CommentedMap) and obj and hasattr(obj, 'lc'):
384+
last_key = list(obj.keys())[-1]
385+
if last_key in obj.lc.data:
386+
last_info = obj.lc.data[last_key]
385387
if len(last_info) >= 4:
386-
val_end_line = last_info[2]
387-
elif isinstance(value, CommentedSeq) and value:
388-
last_idx = len(value) - 1
389-
if last_idx in value.lc.data:
390-
last_info = value.lc.data[last_idx]
388+
# Check if the value is also nested
389+
last_value = obj[last_key]
390+
if isinstance(last_value, (CommentedMap, CommentedSeq)):
391+
# Recurse into nested structure
392+
return find_last_line(last_value)
393+
else:
394+
# Scalar value - return its line
395+
return last_info[2]
396+
elif isinstance(obj, CommentedSeq) and obj and hasattr(obj, 'lc'):
397+
last_idx = len(obj) - 1
398+
if last_idx in obj.lc.data:
399+
last_info = obj.lc.data[last_idx]
391400
if len(last_info) >= 2:
392-
val_end_line = last_info[0]
401+
# Check if the item is also nested
402+
last_item = obj[last_idx]
403+
if isinstance(last_item, (CommentedMap, CommentedSeq)):
404+
# Recurse into nested structure
405+
return find_last_line(last_item)
406+
else:
407+
# Scalar item - return its line
408+
return last_info[0]
409+
return None
410+
411+
last_line = find_last_line(value)
412+
if last_line is not None:
413+
val_end_line = last_line
393414

394415
# Find end of that line
395416
val_end = line_col_to_index(self.original_bytes, val_end_line, 0)

tests/test_ordered_insertions.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,50 @@ def test_insert_key_between_missing_key(tmp_path):
8989
new_key="new",
9090
value=42
9191
)
92+
93+
94+
def test_insert_key_between_with_nested_prev_key(tmp_path):
95+
"""
96+
Test that insert_key_between correctly handles prev_key with nested structures.
97+
98+
Regression test for bug where the last item from a nested structure would
99+
get incorrectly moved into the newly inserted key's value.
100+
"""
101+
yaml_file = tmp_path / "test.yaml"
102+
yaml_file.write_text("""jobs:
103+
unit_tests:
104+
runs-on: ubuntu-latest
105+
strategy:
106+
matrix:
107+
python-version: ["3.11"]
108+
jax-version: ["0.5.2", "0.6.2"]
109+
steps:
110+
- run: echo test
111+
""")
112+
113+
doc = YAYA.load(yaml_file)
114+
115+
# Insert defaults between strategy (which has nested content) and steps
116+
doc.insert_key_between(
117+
"jobs.unit_tests",
118+
prev_key="strategy",
119+
next_key="steps",
120+
new_key="defaults",
121+
value={"run": {"working-directory": "lib/levanter"}}
122+
)
123+
124+
doc.save()
125+
126+
# Verify the structure is correct
127+
doc2 = YAYA.load(yaml_file)
128+
129+
# strategy.matrix should still have both keys
130+
matrix_keys = list(doc2.get_path('jobs.unit_tests.strategy.matrix').keys())
131+
assert matrix_keys == ['python-version', 'jax-version']
132+
133+
# defaults.run should only have working-directory
134+
defaults_run_keys = list(doc2.get_path('jobs.unit_tests.defaults.run').keys())
135+
assert defaults_run_keys == ['working-directory']
136+
137+
# Verify jax-version wasn't moved
138+
assert 'jax-version' not in doc2.get_path('jobs.unit_tests.defaults.run')

0 commit comments

Comments
 (0)