Fix occluded nodes after triangulation become unplaceable in new instances

[ericleonardis]

Fix occluded nodes after triangulation become unplaceable in new instances

Branch: eric/occlusion-skeleton-issue

Summary

After triangulating a frame where a node is visible in only one of two views,
that node can't be triangulated and comes back with no coordinate (null). From
then on every new instance you add is also missing that node — and you can't
click to place / un-occlude it, so you can't build a full-keypoint instance. Saving
.slp + reloading was the only workaround.

This PR makes the "add new instance" path place every skeleton node — filling any
node the template left null with a fallback position and flagging it occluded
(nulledNodes) — so it draws a clickable marker you can un-occlude, instead of a
silent, unreachable null.

Not a skeleton-corruption bug. The report described the node as "deleted from
the skeleton". It isn't — the skeleton keeps all its nodes on every save/reload path.
The node just had no position and no marker, so there was nothing to click. This
is a node-availability bug.

Root cause

A single-view node can't be triangulated → its reprojection is null → the converted
user instance keeps that node null. That null then propagates through the smart-add
machinery:

1. null reprojection stays null in the user instance
   (pose/initialization.js onClonePredictedGroup).
2. The null becomes the smart-add template — recordUserPoints caches points
   preserving nulls, and addNewInstanceSmart seeds each new instance from that cache
   (state.lastUserPoints), so the null node propagates into every new instance.
3. _addNewInstance copied a length-matched template verbatim (ui/interaction.js),
   keeping the null entries.
4. A null node draws no marker, so the un-occlude affordance
   (ui/interaction.js, which toggles instance.nulledNodes on a hit) has nothing to
   hit → no way to place it.

Why save+reload "fixed" it: reload clears the in-memory state.lastUserPoints cache,
so smart-add finds no template and falls through to _addNewInstance's full-topology
layout — which places every node. It discarded the poisoned template rather than fixing
the data.

Regression origin: the smart-add template arrived in #109; the #131 cross-view
tracker changed pose/triangulation.js this weekend and re-exposed the pre-existing
null-propagation path through the #109 template.

The fix

_addNewInstance (ui/interaction.js) no longer copies a length-matched template
verbatim. When a template slot is null, it now:

• fills the slot with a fallback position fanned out from the centroid of the placed
  nodes (spaced so multiple nulls don't overlap), and
• records the index in a Set that becomes instance.nulledNodes

— the same treatment _convertToUserInstance already gives its null nodes. An
all-null template falls through to the full-topology layout. Net invariant:

An added instance has a position (and a clickable marker) for every skeleton node,
with occluded ones flagged in nulledNodes rather than left as unreachable nulls.

So a node that was occluded/untriangulatable in the source instance is now added as an
occluded-but-positioned node you can click to un-occlude, instead of disappearing.

Crucially, filled-occluded nodes flagged in nulledNodes are excluded from the 3D
solve (triangulation already skips nulledNodes), so the fanned-out placeholder never
pollutes triangulation.

Files changed

File	Change
ui/interaction.js	_addNewInstance fills null template slots (centroid fan-out) and flags them nulledNodes; all-null template falls back to topology layout. (+43/-3)
tests/test-occlusion-node-availability.js	New test suite (see below).
tests/test-runner.html	Register the new test file.

Tests

tests/test-occlusion-node-availability.js builds an InteractionManager (like
test-assignment.js) plus a triangulation integration case:

• Core (pins the bug): a null template node must not become an unreachable
  null — it's placed and flagged nulledNodes. (Failed before the fix.)
• No regression: a full template round-trips unchanged; a no-template add
  topology-places every node (the save+reload path).
• No cascade: the created instance has no null points, so it can't re-poison the
  next smart-add template.
• Multiple nulls each get a distinct (non-overlapping) placeholder and are all flagged.
• All-null template falls back to full topology (no crash).
• Triangulation exclusion (key interaction): a filled-occluded node flagged in
  nulledNodes yields points3d[k] == null (excluded from the 3D solve), while the real
  node still triangulates.

Full suite: 1067/1067 passing.

Confirmed: the skeleton is never mutated

A full read of every save/reload path confirms no code removes a node from
session.skeleton.nodes in the occlusion/triangulation/add-instance/save/reload flow
(the only true node-deletion is the info-panel "×" button). All export paths serialize
the full node list, reload rebuilds from it, and nulledNodes round-trips. What persisted
was the null/nulled point in instances and the inherited smart-add template — both
addressed here.

Related latent hazards (noted, not fixed here)

• propagateNodeAdded / propagateNodeRemoved don't touch reprojectedInstances
  (pose/pose-data.js), so after a skeleton node add/remove the reprojected instances
  keep a stale-length points array.
• Triangulation takes the node count from inst.points.length, not the skeleton
  (pose/triangulation.js). Worth making the point-array-length ↔ skeleton.nodes.length
  invariant explicit in a follow-up.

This addresses issue #144

[github-actions]

Preview Deployment

Preview URL	https://talmolab.github.io/luc3d/pr/146/
Commit	66122cc

This preview will be removed when the PR is closed.

[ericleonardis]

Fix: occlusions on unlinked user labels now survive the project .slp round-trip

The bug

Occlude a node on an unlinked user label — e.g. a prediction converted to a user label that was never grouped/linked — then Save As a project .slp and reopen it. The point came back positioned but no longer occluded. (Grouped/linked instances were unaffected.)

Root cause

The occlusion flag (nulledNodes) is only persisted in per-group metadata (metadata.lucid.instanceMeta). An ungrouped instance has no group, so the flag was never written.

The occlusion was still in the file, though: _buildSioPoints saves an occluded node as real xy + visible:false. The load side just never turned that "finite-but-invisible" point back into nulledNodes — it only rebuilt the flag from the (missing) group metadata.

The fix

nulledNodesFromOcclusion(points, occluded, type) reconstructs the flag from the finite-but-invisible signal, for user instances only (an invisible predicted point is low-confidence, not a user occlusion). It runs in the pass-1 raw-instance build of both reopen paths:

• handleLoadSlpFile (File → Load SLP)
• handleLoadSessionFolderSingleSlp (Load Session Folder)

Grouped instances are unaffected — their pass-1 copy is still replaced by the metadata-driven reconstruction. The helper lives in the dependency-free import-export/import-track-resolve.js so it can be unit-tested in the in-browser runner.

Testing

• E2e round-trip (tests/e2e/occlusion-roundtrip.mjs): builds a project with an occluded node, saves to .slp via the real save path, assembles a session folder (project.slp + calibration + videos), and reopens through the real load path. Covers both grouped and unlinked scenarios.
  • Verified it fails pre-fix on the unlinked case (nulledNodes: null) and passes with it (nulledNodes: [1], position preserved).
• Unit tests (tests/test-occlusion-derive.js, 8 assertions): user occlusion derivation, missing-point exclusion, predicted-instance exclusion, and the no-op cases. In-browser suite: 1096 → 1104, all passing.

Commits

• 8cfaca2 — grouped / session-folder reopen fix (shared restoreGroupingAndUnlink)
• fb68fc0 — e2e round-trip test
• a89d7ac — unlinked user-label occlusion fix
• 686ce6a — unit test + relocate helper to a bridgeable module (behavior-identical)