Skip to content

fix: preserve existing keys on dotted list-element writes#310

Open
gaoflow wants to merge 1 commit into
cdgriffith:masterfrom
gaoflow:fix-box-dots-list-overwrite
Open

fix: preserve existing keys on dotted list-element writes#310
gaoflow wants to merge 1 commit into
cdgriffith:masterfrom
gaoflow:fix-box-dots-list-overwrite

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 20, 2026

Copy link
Copy Markdown

Summary

With default_box=True and box_dots=True, writing a second dotted key into an already-existing list element silently discards the keys already written there:

from box import Box
b = Box(default_box=True, box_dots=True)
b["a[0].x"] = 1     # {'a': [{'x': 1}]}
b["a[0].y"] = 2
b.to_dict()         # {'a': [{'y': 2}]}   <-- 'x' is gone, expected {'a': [{'x': 1, 'y': 2}]}

The write succeeds but drops previously-stored data — a silent correctness/data-loss bug. (Different indices like a[0].x then a[1].y are fine; the bug is a second write to the same index.)

Cause

BoxList.__setitem__ (box/box_list.py), in the default_box branch, unconditionally replaces the element at the target index with a fresh empty Box/BoxList before recursing into it — even when a populated container already lives there. So the second write to a[0] throws away the Box({'x': 1}) already stored and recurses into a brand-new empty one.

The sibling Box.__setitem__ already guards this with an "is it already the right container?" check; the BoxList path was missing it.

Fix

Read the current element first and only replace it when it isn't already the right container type. A scalar (or None placeholder from list extension) is still correctly overwritten; an existing Box/BoxList is preserved and recursed into.

Added regression coverage to test_box_list_default_dots (two keys into one element, nested list-in-list, and the scalar-overwrite case). Full suite: 154 passed; the 5 test_toon_* failures are pre-existing (a NotImplementedError/BoxError from the third-party toon_format package) and reproduce identically on a clean master. black --config=.black.toml --check clean.

This pull request was prepared with the assistance of AI, under my direction and review.

BoxList.__setitem__ unconditionally replaced the element at the target
index with a fresh empty container before recursing, so a second dotted
write into an existing element (a[0].y after a[0].x) discarded the keys
already stored there. Only replace the element when it is not already the
right container type, matching the guard in Box.__setitem__.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant