Fix cross-screen state isolation and element restoration

src/common/model/optics/BaseElement.ts

@@ -56,6 +56,13 @@ export abstract class BaseElement implements OpticalElement {
    */
   public reassignIdForDeserialization(id: string): void {
     this._id = id;
+    const match = /^element-(\d+)$/.exec(id);
+    if (match) {
+      const restoredSequence = Number(match[1]);
+      if (Number.isSafeInteger(restoredSequence)) {
+        nextElementId = Math.max(nextElementId, restoredSequence + 1);
+      }
+    }
   }
 
   /**

src/common/model/optics/OpticalElementPhetioObject.ts

@@ -2,6 +2,7 @@
  * PhET-iO instrumented wrapper for one {@link OpticalElement} in a {@link PhetioGroup}.
  */
 
+import { Emitter } from "scenerystack/axon";
 import type { Tandem } from "scenerystack/tandem";
 import { IOType, ObjectLiteralIO, PhetioObject } from "scenerystack/tandem";
 import OpticsLabNamespace from "../../../OpticsLabNamespace.js";
@@ -12,6 +13,8 @@ type ElementStateRecord = Record<string, unknown>;
 
 export default class OpticalElementPhetioObject extends PhetioObject {
   public opticalElement: OpticalElement;
+  /** Fires when PhET-iO state replaces the wrapped model object. */
+  public readonly opticalElementReplacedEmitter = new Emitter<[OpticalElement, OpticalElement]>();
 
   public constructor(tandem: Tandem, stateOrLive: ElementStateRecord) {
     const live = stateOrLive[LIVE_ELEMENT_STATE_KEY] as OpticalElement | undefined;
@@ -37,6 +40,7 @@ export default class OpticalElementPhetioObject extends PhetioObject {
 
   public override dispose(): void {
     this.opticalElement.dispose();
+    this.opticalElementReplacedEmitter.dispose();
     super.dispose();
   }
 
@@ -50,12 +54,14 @@ export default class OpticalElementPhetioObject extends PhetioObject {
       toStateObject: (obj) => ({ ...obj.opticalElement.serialize(), id: obj.opticalElement.id }),
       stateObjectToCreateElementArguments: (state: ElementStateRecord) => [state],
       applyState: (obj, state) => {
-        obj.opticalElement.dispose();
         const el = deserializeElement(state);
         if (!el) {
           throw new Error("OpticalElementPhetioObject.applyState: invalid element state");
         }
+        const previousElement = obj.opticalElement;
         obj.opticalElement = el;
+        obj.opticalElementReplacedEmitter.emit(previousElement, el);
+        previousElement.dispose();
       },
     },
   );

src/common/model/optics/OpticsScene.ts

@@ -91,6 +91,8 @@ export class OpticsScene extends PhetioObject {
   public readonly opticalElementsGroup: PhetioGroup<OpticalElementPhetioObject, [Record<string, unknown>]>;
 
   public readonly sceneChangedEmitter: Emitter;
+  /** Fires when PhET-iO state replaces an element object inside its stable group wrapper. */
+  public readonly elementReplacedEmitter = new Emitter<[OpticalElement, OpticalElement]>();
 
   /** Undo/redo history for add/remove element commands. */
   public readonly history: CommandHistory = new CommandHistory();
@@ -216,11 +218,24 @@ export class OpticsScene extends PhetioObject {
       },
     );
 
-    this.opticalElementsGroup.elementCreatedEmitter.addListener(() => {
+    this.opticalElementsGroup.elementCreatedEmitter.addListener((wrapper) => {
+      this._elementById.set(wrapper.opticalElement.id, wrapper.opticalElement);
+      wrapper.opticalElementReplacedEmitter.addListener((previousElement, replacementElement) => {
+        if (this._elementById.get(previousElement.id) === previousElement) {
+          this._elementById.delete(previousElement.id);
+        }
+        this._elementById.set(replacementElement.id, replacementElement);
+        this.invalidate();
+        this.sceneChangedEmitter.emit();
+        this.elementReplacedEmitter.emit(previousElement, replacementElement);
+      });
       this.invalidate();
       this.sceneChangedEmitter.emit();
     });
-    this.opticalElementsGroup.elementDisposedEmitter.addListener(() => {
+    this.opticalElementsGroup.elementDisposedEmitter.addListener((wrapper) => {
+      if (this._elementById.get(wrapper.opticalElement.id) === wrapper.opticalElement) {
+        this._elementById.delete(wrapper.opticalElement.id);
+      }
       this.invalidate();
       this.sceneChangedEmitter.emit();
     });
@@ -239,6 +254,9 @@ export class OpticsScene extends PhetioObject {
    */
   public addElement(element: OpticalElement, recordHistory = true): void {
     const doAdd = (): void => {
+      if (this._elementById.has(element.id)) {
+        throw new Error(`Cannot add duplicate optical element id "${element.id}"`);
+      }
       this.opticalElementsGroup.createNextElement({
         ...element.serialize(),
         id: element.id,

src/common/view/RayTracingCommonView.ts

@@ -32,6 +32,7 @@ import { RayPropagationView } from "./RayPropagationView.js";
 import { sceneHistoryRegistry } from "./SceneHistoryRegistry.js";
 import { downloadSceneSVG } from "./SceneSVGExporter.js";
 import { ToolsPanel } from "./ToolsPanel.js";
+import { trackRegistry } from "./TrackRegistry.js";
 import { ViewOptionsModel } from "./ViewOptionsModel.js";
 import { viewSnapState } from "./ViewSnapState.js";
 
@@ -247,11 +248,19 @@ export class RayTracingCommonView extends ScreenView {
     super(options);
 
     this.model = model;
-    sceneHistoryRegistry.setHistory(model.scene.history);
     const tandem = options?.tandem;
     const uiStrings = StringManager.getInstance().getUIStrings();
 
     this.viewOptions = new ViewOptionsModel(tandem?.createTandem("viewOptions"));
+    this.visibleProperty.link((visible) => {
+      if (visible) {
+        sceneHistoryRegistry.setHistory(model.scene.history);
+        trackRegistry.setActiveScope(this.viewOptions.scopeId);
+      } else {
+        sceneHistoryRegistry.clearHistory(model.scene.history);
+        trackRegistry.clearActiveScope(this.viewOptions.scopeId);
+      }
+    });
 
     // ── Model-View Transform ────────────────────────────────────────────────
     // Maps model origin (0, 0) to the centre of the visible play area.
@@ -509,6 +518,33 @@ export class RayTracingCommonView extends ScreenView {
       }
     });
 
+    // PhET-iO can replace the model object inside an existing group wrapper.
+    // Rebuild the corresponding view so editing, rendering, and lookup all
+    // continue to reference the same replacement object.
+    model.scene.elementReplacedEmitter.addListener((previousElement, replacementElement) => {
+      if (this.selectedElementProperty.value === previousElement) {
+        this.selectedElementProperty.value = null;
+      }
+      const previousView = this.elementViewMap.get(previousElement.id);
+      if (previousView) {
+        this.elementViewMap.delete(previousElement.id);
+        previousView.dispose();
+      }
+      const previousTandem = this.elementTandemMap.get(previousElement.id);
+      if (previousTandem) {
+        RayTracingCommonView._cleanupElementTandem(previousTandem);
+        this.elementTandemMap.delete(previousElement.id);
+      }
+
+      const tn = replacementElement.id.replace(/-(\d+)$/, (_, n: string) => n);
+      const et = tandem?.createTandem(tn) ?? Tandem.OPTIONAL;
+      const replacementView = createOpticalElementView(replacementElement, modelViewTransform, et, this.viewOptions);
+      if (replacementView) {
+        this.elementTandemMap.set(replacementElement.id, et);
+        this._setupView(replacementElement, replacementView);
+      }
+    });
+
     // ── Tools Panel ───────────────────────────────────────────────────────────
     // ToolsPanel owns the measuring tape, protractor, all toggle checkboxes,
     // the ray-density control, and the accordion box.  It pins the accordion to
@@ -627,6 +663,9 @@ export class RayTracingCommonView extends ScreenView {
     // clicking the trash icon), but only when no text input has focus.
     // Stored as a class field so it can be removed in dispose().
     this._handleKeyDown = (event: KeyboardEvent): void => {
+      if (!this.isVisible()) {
+        return;
+      }
       const target = event.target as HTMLElement;
       const isTextInput = target.tagName === "INPUT" || target.tagName === "TEXTAREA" || target.isContentEditable;
 
@@ -700,6 +739,8 @@ export class RayTracingCommonView extends ScreenView {
 
   public override dispose(): void {
     window.removeEventListener("keydown", this._handleKeyDown);
+    sceneHistoryRegistry.clearHistory(this.model.scene.history);
+    trackRegistry.clearActiveScope(this.viewOptions.scopeId);
     super.dispose();
     this.viewOptions.dispose();
   }

src/common/view/SceneHistoryRegistry.ts

@@ -6,7 +6,8 @@
  * threading the reference through every call site.
  *
  * Lifecycle:
- *   RayTracingCommonView calls setHistory(model.scene.history) during construction.
+ *   A visible RayTracingCommonView registers its model.scene.history and
+ *   conditionally clears it when that screen becomes inactive.
  *   Drag and slider helpers call sceneHistoryRegistry.history to get the
  *   CommandHistory instance (null-safe: no-op when history is not set).
  *
@@ -19,11 +20,18 @@ import type { CommandHistory } from "../model/optics/CommandHistory.js";
 class SceneHistoryRegistryImpl {
   private _history: CommandHistory | null = null;
 
-  /** Called once by RayTracingCommonView during construction. */
+  /** Set the history for the currently visible screen. */
   public setHistory(history: CommandHistory | null): void {
     this._history = history;
   }
 
+  /** Clear a screen's history only if it is still the active registration. */
+  public clearHistory(history: CommandHistory): void {
+    if (this._history === history) {
+      this._history = null;
+    }
+  }
+
   /** Returns the active CommandHistory, or null when not wired up. */
   public get history(): CommandHistory | null {
     return this._history;

src/common/view/TrackRegistry.ts

@@ -8,25 +8,39 @@
 import type { Point } from "../model/optics/Geometry.js";
 
 export interface TrackDescriptor {
+  scopeId: string;
   getP1: () => Point;
   getP2: () => Point;
 }
 
 class TrackRegistryImpl {
   private readonly tracks = new Map<string, TrackDescriptor>();
+  private activeScopeId: string | null = null;
 
-  public register(id: string, getP1: () => Point, getP2: () => Point): void {
-    this.tracks.set(id, { getP1, getP2 });
+  public register(id: string, scopeId: string, getP1: () => Point, getP2: () => Point): void {
+    this.tracks.set(id, { scopeId, getP1, getP2 });
   }
 
   public unregister(id: string): void {
     this.tracks.delete(id);
   }
 
+  public setActiveScope(scopeId: string): void {
+    this.activeScopeId = scopeId;
+  }
+
+  public clearActiveScope(scopeId: string): void {
+    if (this.activeScopeId === scopeId) {
+      this.activeScopeId = null;
+    }
+  }
+
   public getAllTracks(): ReadonlyArray<{ id: string; p1: Point; p2: Point }> {
     const result: Array<{ id: string; p1: Point; p2: Point }> = [];
     for (const [id, desc] of this.tracks) {
-      result.push({ id, p1: desc.getP1(), p2: desc.getP2() });
+      if (desc.scopeId === this.activeScopeId) {
+        result.push({ id, p1: desc.getP1(), p2: desc.getP2() });
+      }
     }
     return result;
   }

src/common/view/ViewOptionsModel.ts

@@ -22,7 +22,11 @@ import { RAY_STUB_LENGTH_MAX_PX, RAY_STUB_LENGTH_MIN_PX } from "../../OpticsLabC
 import OpticsLabNamespace from "../../OpticsLabNamespace.js";
 import opticsLabQueryParameters from "../../preferences/opticsLabQueryParameters.js";
 
+let nextViewScopeId = 1;
+
 export class ViewOptionsModel {
+  /** Identifies the owning screen for process-wide interaction registries. */
+  public readonly scopeId = `view-scope-${nextViewScopeId++}`;
   public readonly handlesVisibleProperty: BooleanProperty;
   public readonly focalMarkersVisibleProperty: BooleanProperty;
   public readonly rayArrowsVisibleProperty: BooleanProperty;

src/common/view/guides/TrackView.ts

@@ -112,6 +112,7 @@ export class TrackView extends BaseOpticalElementView {
     // Register with the track registry for snap logic.
     trackRegistry.register(
       track.id,
+      viewOptions?.scopeId ?? "unscoped",
       () => track.p1,
       () => track.p2,
     );

tests/memory-leak.test.ts

@@ -609,6 +609,7 @@ describe("Memory leak regression", () => {
     const trackId = el.id;
 
     const view = createOpticalElementView(el, mvt, Tandem.OPT_OUT, viewOptions);
+    trackRegistry.setActiveScope(viewOptions.scopeId);
 
     // Immediately after construction the track must appear in the registry.
     const registeredBefore = trackRegistry.getAllTracks().some((t) => t.id === trackId);
@@ -620,6 +621,7 @@ describe("Memory leak regression", () => {
     // After disposal the entry must be gone.
     const registeredAfter = trackRegistry.getAllTracks().some((t) => t.id === trackId);
     expect(registeredAfter).toBe(false);
+    trackRegistry.clearActiveScope(viewOptions.scopeId);
   });
 
   // ── Multiple sequential scenes do not cross-retain each other's elements ──