Skip to content

Fix TwoWayDict reverse map going stale on key reassignment#6551

Open
c-tonneslan wants to merge 2 commits into
Textualize:mainfrom
c-tonneslan:fix/twowaydict-reassign
Open

Fix TwoWayDict reverse map going stale on key reassignment#6551
c-tonneslan wants to merge 2 commits into
Textualize:mainfrom
c-tonneslan:fix/twowaydict-reassign

Conversation

@c-tonneslan
Copy link
Copy Markdown

TwoWayDict.__setitem__ writes the new value into _forward and _reverse, but never removes the old value from _reverse when a key is reassigned:

d = TwoWayDict({})
d[1] = 10
d[1] = 20
d.get(1)               # 20  ✓
d.get_key(20)          # 1   ✓
d.get_key(10)          # 1   ✗ should be None
d.contains_value(10)   # True ✗ should be False

So _reverse stops being a true inverse of _forwardget_key and contains_value keep reporting a value the key no longer maps to.

The fix drops the old value's reverse entry when the key already exists. The pre-existing TODO about duplicate values (two keys → one value) is a separate design question and is left untouched.

Added test_set_item_reassigning_key_clears_old_reverse_entry alongside the existing test_set_item.

Worth noting on scope: DataTable (the in-tree user of TwoWayDict) currently rebuilds these dicts rather than reassigning keys, so this isn't a user-visible bug today — it's a correctness hole in the utility class itself, which has an explicit two-way invariant and its own test file.

Charlie Tonneslan added 2 commits May 21, 2026 07:33
TwoWayDict.__setitem__ wrote the new value into both maps but never
removed the old value from the reverse map. Reassigning an existing
key, e.g. d[1] = 10 then d[1] = 20, left _reverse with both 10 and 20
pointing back at 1, so get_key(10) and contains_value(10) kept
reporting the stale value.

Drop the old value's reverse entry when a key already exists. (The
existing TODO about duplicate *values* is a separate question and is
left as-is.)

Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>
Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>
@willmcgugan
Copy link
Copy Markdown
Member

Your PR has been closed due to a AI policy violation.

Please read the following before submitting further PRs.

https://github.com/Textualize/textual/blob/main/AI_POLICY.md

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.

2 participants