fix(dsl): reject duplicate entity ids instead of splitting the scene

[dchaudhari7177]

Closes #96

Taking you up on the "happy to send it as its own PR" offer — and thanks for keeping it out of the #41 branch, that was the right call.

The fix

build() merges _entities and _distractors into one spec list and keys frames_by_id by id, so the two share a namespace whether or not the API says so. A duplicate left the scene inconsistent with itself: both entries reached scene.entities and the manifest, while frames_by_id kept only the last one written.

Guarded in entity() and distractor() rather than in build(), as you suggested — at the call site the error can name the id and say what already holds it, which is the thing the caller can act on:

ValueError: duplicate entity id 'thing': already added as a distractor, cannot add it
again as an entity. Entities and distractors share one id namespace because the scene
keys its per-frame lookup by id.

The check spans both lists, so the entity("obj2") / distractor("obj2") collision you noted is caught too.

randomize() is unaffected

Its generated ids come from the per-builder counter, so its own calls can't collide. A caller who hand-adds rand_distractor_0 and then calls randomize() now gets an error — which seems like the right outcome, since that collision previously produced exactly the split described above. Flagging it in case you'd rather randomize() skipped ids already taken.

This is a public behaviour change

Silent acceptance → ValueError, which is your call to make. Nothing in the suite depended on the old behaviour: 360 tests pass unchanged with the guard in place.

7 new tests in tests/test_entity_ids.py:

test	covers
two distractors, one id	your exact repro
two entities, one id	same-kind, other list
entity then distractor	the cross-list case
distractor then entity	the other order
the error names the prior holder	message quality
rejection leaves no half-added spec	build() after a caught error
distinct ids still agree with the manifest	the invariant being protected

6 of the 7 fail without the guard. The 7th is the invariant test, which passes on main by construction.

Verification

pytest, ruff check, ruff format --check and mypy src all clean.

One caveat: test_dsl_breadth.py, test_dsl_scene.py, test_mtmc.py, test_rig_heights.py and test_smoke.py could not be collected in my environment — they import multicam_occlusion, which isn't part of this repo — so they're excluded from the counts above and CI is what checks them. If any of those builds a scene with a deliberately duplicated id, this will surface there and I'll fix it.

---

🤖 Generated with Claude Code