Skip to content

SceneBuilder silently accepts duplicate entity ids: the manifest keeps both, frames_by_id keeps only the last #96

Description

@Nitjsefnie

Description

SceneBuilder accepts two entities with the same id, and the result is inconsistent between the manifest and the frame lookup: both entries survive into scene.entities and into the built manifest, while frames_by_id keeps only the second.

Reproduced on main @ 3aba5bb:

SceneBuilder(fps=10.0, num_frames=5).cameras(...).entity("obj", ...) \
    .distractor("dup", Path.linear((1, 1, 0), (1, 1, 0))) \
    .distractor("dup", Path.linear((9, 9, 0), (9, 9, 0))).build()

gives

entity ids:      ['obj', 'dup', 'dup']
manifest ids:    ['obj', 'dup', 'dup']
frame-0 centers: [[1.0, 1.0, 0.0], [9.0, 9.0, 0.0]]

An entity() and a distractor() sharing an id collide the same way:

.entity("obj2", ...).distractor("obj2", ...).build()
→ entity ids: ['obj', 'obj2', 'obj2']

Nothing raises and nothing warns. src/multicam_sim/dsl/builder.py:235 is frames_by_id[spec.id] = frames, so the second entity replaces the first in that dict, while the entity list and the manifest retain both. Every id-keyed consumer downstream reads the survivor — attachments, and the annotation and group paths that index frames_by_id — so those see one entity while the manifest a consumer parses contains two with the same id.

Neither entity() (builder.py:79) nor distractor() (builder.py:100) checks that the id is unused.

Expected Behavior

A duplicate entity id is rejected at the point it is added, with an error naming the id — or, if duplicates are meant to be allowed, the manifest and frames_by_id agree on what the scene contains.

Reproduction Steps

  1. Check out main and uv sync --dev.
  2. Build a scene calling .distractor("dup", ...) twice with different paths, as above.
  3. Inspect scene.entities and the built manifest — dup appears twice.
  4. Inspect the frames keyed by "dup" — only the second path is present.

Environment / Context

  • main @ 3aba5bb, verified in a clean worktree with the package imported from that worktree (asserted via multicam_sim.__file__)
  • Python 3.13, Debian 13 (x86_64)

Discovered During

Found while implementing #41 (seeded domain randomization). The randomization work generated ids of its own and could reach this collision, which is fixed on that branch by making its generated ids unique; this issue is about the underlying builder behaviour, which is independent of that change and reproduces on main with no randomization involved.

Suggested Fix

Unverified: a uniqueness check in entity() and distractor() (both funnel into the same spec list) would catch it at the call site, where the error can name the offending id, rather than at build() where the origin is lost. I have deliberately not touched either method in the #41 branch — a guard there changes existing public behaviour and is your call, not something to bury in a feature PR. Happy to send it as its own PR if you want it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions