[dmccoystephenson]

Summary

• Location::removeEntity now erases every pointer whose id matches, instead of only the first. Grid::removeEntity, Environment::removeEntity, and the move methods all go through it, so an entity added twice to one location is fully removed and a move leaves a single pointer at the destination.
• Test 66 previously pinned the leftover pointer. It now asserts the fixed result, and its function is renamed to match. Tests 69-71 are added.
• The README paragraph that described the old behaviour is rewritten.

Design choice

The issue offered two options. The fix was placed in Location::removeEntity rather than in a repeat loop in Grid::removeEntity, so that a direct Location::removeEntity call gets the same guarantee: the entity is no longer present afterwards.

One consequence: because matching is by id, a distinct same-id twin stored in that location is erased too, and the twin keeps its ids. That agrees with the README's "Entity Identity" section, which treats such objects as the same entity, and it is pinned by Test 71.

Test plan

• CI "Library tests" failed on the tests-only commit a3463cd, aborting on the Test 66 assertion at src/tests.cpp:1100 (exit 134)
• CI "Library tests" passed on the fix commit 0d69d11, with 71 of 71 registered tests reporting success
• Strata frontend and browser build jobs passed on the fix commit
• run_tests_asan.sh was not run: CI has no AddressSanitizer job, and local execution is denied. The change erases vector elements and alters no allocation or object lifetime.

Local execution of tests_executable is denied in this session, so CI is the test anchor.

Closes #70

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Self-review rubric (head 0d69d11):

• Scope: PASS — the diff touches only src/location.cpp, src/tests.cpp, and README.md, each needed for Environment::removeEntity leaves an entity present when it was added twice to one location #70.
• Tests-new: PASS — no public method was added; the changed behaviour is covered by Tests 66, 69, 70, and 71.
• Tests-fix: PASS, with a limit — a local stash-and-run is denied, so the check was done on CI: the tests-only commit a3463cd aborted at src/tests.cpp:1100 and the fix commit passed. The abort stopped the run at Test 66, so Tests 69-71 were observed passing with the fix but were not individually observed failing without it.
• Sibling structure: PASS — new tests follow the existing void test<Behavior>() and Test N - ... --- Success pattern. No new files.
• Sibling renames: PASS — the only rename is the Test 66 function, which has no parallel sibling.
• Docs: PASS — the README "Removal takes out..." paragraph was rewritten, and the class table is unaffected.
• Issue resolution: PASS — Environment::removeEntity leaves an entity present when it was added twice to one location #70's named surface is fixed at its root, and both reported symptoms (the leftover pointer on removal, and the double storage after moveEntityToNewLocation) are asserted by Tests 66 and 70.
• Local test suite: PASS via CI — Library tests (make + CMake) reported 71 successes against 71 registered tests. The Strata and browser jobs are also green.
• Header/definition parity: PASS — no header changed.
• Include guards: PASS — no new header.
• Test registration: PASS — all three new tests are called from main(), and the count of 71 matches.

Judgment call flagged: this is a behaviour change for same-id twins stored in one location. Previously only the first match was erased; now both are, and the twin keeps stale ids. This is consistent with the README's id-matching rule and is pinned by Test 71, so it was not treated as blocking.

src/location.cpp:33 — the [entity] capture dereferences the caller's pointer on every comparison, which is safe because remove_if moves pointers only and never destroys an Entity.

Gap: run_tests_asan.sh was not run, since CI has no AddressSanitizer job and local execution is denied. The change affects no allocation or object lifetime, so that script's expectation does not apply here.

This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Correction to the self-review above: the remove_if note belongs at src/location.cpp:36, not src/location.cpp:33. The rubric results are unchanged.

This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson