fix: Error after importing a reticular similar to an existing one and…

[aiAdrian]

… clicking on a node fixed

Analysis

The error occurred sporadically, and until now no user was able to report it in a reproducible way.
I have now understood and identified the root cause.

Issue is here

netzgrafik-editor-frontend/src/app/view/editor-main-view/data-views/nodes.view.ts

Line 731 in a65706e

this.editorView.trainrunSectionPreviewLineView.stopPreviewLine();

check is wrong (unsafe)

There is a startNode not eq undefined thus the check is not true when an object copy will be returned (start or end node)

If (startNode === endNode) {

check must be (safe)

The ID check itself is fine. But to ensure that the objects are valid we have also to check that null or undefined values are handled correctly

if (!startNode || !endNode || startNode?.getId() === endNode?.getId()) {

fix

to ensure that also the check with the source / target node can not cause another issue we fix it as well

    if (startNode?.getId() === endNode?.getId()) {
      return;
    }
    if (existingTrainrunSection !== null) {
      if (
        existingTrainrunSection.getSourceNodeId() === startNode?.getId() &&
        existingTrainrunSection.getTargetNodeId() === endNode?.getId()
      ) {
        return;
      }
      if (
        existingTrainrunSection.getSourceNodeId() === endNode?.getId() &&
        existingTrainrunSection.getTargetNodeId() === startNode?.getId()
      ) {
        return;
      }
    }

[emersion]

This PR seems to workaround the problem, rather than fixing the root cause. There are stale cached objects held by d3, and these aren't properly nuked when loading a new DTO.

[louisgreiner]

I'm not getting the matter of the issue yet, but I'll keep an eye on it.

[aiAdrian]

This PR seems to workaround the problem, rather than fixing the root cause. There are stale cached objects held by d3, and these aren't properly nuked when loading a new DTO.

      # code sample to show where "callbacks informations comes from ... "
      .on("mouseover", (n: NodeViewObject) => this.onHoverNodeMouseover(n.node))
      .on("mouseout", (n: NodeViewObject) => this.onHoverNodeMouseout(n.node))
      .on("mousedown", (n: NodeViewObject) => this.onNodeMousedown(n.node))
      .on("mouseup", (n: NodeViewObject) => this.onNodeMouseup(n.node))
      .on("dblclick", (n: NodeViewObject) => this.onNodeDetailsClicked(n.node));

Each rendering object (d3 / svg) has an element attached e.g. NodeViewObject - in this case the node - ref (to a Node object). This will only be updated when the node object has changed. This will be detected

NodeViewObject.generateKey -> string

changes. This can lead that the object comparison will no longer correct

If (startNode === endNode) {

because the object has changed in the e.g nodeService but not yet in the rendering. Then the ref is no longer equal -> this leads to side-effects. The isn't a d3.js issue, this is by design a problem we encount with Netzgrafik-Editor as a real-time visualisation tool. The optimisation leads to make things more complicated. Thus we have to ensure in some cased that we really compare what we like to compare -> node.id 's and not objects!

Remember: Once the scenario gets changes (replaced) the update of the rendering will only be done when

NodeViewObject.generateKey -> string 

has changed.

[aiAdrian]

It found by hazard this duplicated code. While testing to "clear" all object during load which could overcome the problem. But this leads to more complicated things to do. This was the reason i stop changed the loading stuff.

I think we have just to ensure when then callbacks gets correctly handled. Once we face a comparison by object in the code where we compare "rendering system stored / cached objects" with ...Service stored we should focus on id based comparision not on objects. Thus we haven't to change the loading stuff. This problem was located at wrong comparision logic.

[aiAdrian]

@louisgreiner
@emersion

fix: Force D3 rebinding on import to prevent stale object references (#851)

Implement load counter mechanism to resolve issue where clicking a node
after re-importing a netzgrafik creates invalid trainruns starting and
ending at the same node.

Root Cause:
D3.js key-based data binding reuses event callbacks for unchanged nodes.
After DTO import, new Node objects are created but D3's update pattern
keeps old callbacks with stale object references from pre-import nodes.

Solution:

• Add netzgrafikLoadCounter to DataService that increments on user imports
• Expose counter through service chain: DataService → NodeService →
  EditorView → all ViewObjects
• Prefix all D3 keys with "LC{counter}" to force complete rebinding when
  counter changes
• User imports (file load, backend load) increment counter
• Internal operations (undo/redo, preview) preserve counter for performance

Changes:

• DataService: Added loadCounter field, getNetzgrafikLoadCounter() method,
  incrementLoadCounter parameter to loadNetzgrafikDto()
• NodeService: Added getNetzgrafikLoadCounter() pass-through
• EditorView: Added getNetzgrafikLoadCounter() pass-through
• All ViewObjects (Node, Note, Connection, TrainrunSection, Transition):
  Added "LC" + loadCounter prefix to generateKey()
• Updated import paths in app.component, editor-tools-view, and
  version-control.service with incrementLoadCounter=true
• Updated all test files: fixed ViewObject key expectations and
  EditorMainViewComponent constructor calls

Impact:
User imports now force D3 to recreate all bindings with fresh object
references, preventing stale reference bugs. Undo/redo maintains
performance by not triggering rebinding.

[aiAdrian]

This change ensures that even in the worst-case scenario—if the rendering object based on the load counter malfunctions—this check would still be handled correctly. The old check should still be compared to the new, correct one. However, the issue of outdated objects in the rendering (SVG) is resolved by the counter.

Therefore, I recommend not reverting this change.

[aiAdrian]

This load counter is the most important new "thing" I have introduced. The load counter ensures that during runtime each loadNetzgrafikDto will get a new LC and thus the rendering gets updated through ...ViewObjects.key --> change

[aiAdrian]

This PR seems to workaround the problem, rather than fixing the root cause. There are stale cached objects held by d3, and these aren't properly nuked when loading a new DTO.

I introduced the Load Counter -> this resolves the problem. It feels the workaround should be fixed.

[louisgreiner]

This indeed fixes the bug as I tested, I'm not aware of what the solution should be. This implementation looks like a working hack. I don't know if you had something else in mind @emersion?

[aiAdrian]

I’m not sure whether this really counts as a hack — but I can’t rule it out either. In any case, it’s definitely a solid basis for further discussion. A short workshop would make sense.

@emersion
@louisgreiner