Skip to content

Commit 7e12451

Browse files
talmoclaude
andauthored
Fix py/id resolution bug in SLP skeleton decoder (#257)
* Add failing test for SLP edge type py/id resolution bug Add test_slp_decoder_edge_type_pyid_resolution to catch a bug where py/id references in edge types are treated as direct edge type values instead of references to previously defined edge types. The bug causes edges and symmetries to be swapped when a symmetry edge (EdgeType=2) is defined before a regular edge (EdgeType=1) in the metadata. Test uses exact metadata from a real .slp file and currently fails as expected. The fix will be applied to SkeletonSLPDecoder.decode() in sleap_io/io/skeleton.py. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix py/id resolution bug in SLP skeleton decoder Fix bug in SkeletonSLPDecoder where py/id references in edge types were treated as direct edge type values instead of references to previously defined edge types. The bug caused edges and symmetries to be swapped when a symmetry edge (EdgeType=2) was defined before a regular edge (EdgeType=1), because: - First py/reduce creates EdgeType(2) and gets py/id=1 - Second py/reduce creates EdgeType(1) and gets py/id=2 - Old code: py/id=1 was treated as EdgeType(1) (wrong!) - Fixed code: py/id=1 correctly resolves to EdgeType(2) Implementation uses single-pass processing that builds the py/id mapping as links are processed, then looks up references. Falls back to treating py/id as direct edge type value for files without py/reduce definitions (maintains backward compatibility). Fixes test_slp_decoder_edge_type_pyid_resolution and resolves issues with real .slp files that have non-standard edge type ordering. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Apply ruff formatting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Remove unreachable edge case --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6ba4849 commit 7e12451

2 files changed

Lines changed: 125 additions & 2 deletions

File tree

sleap_io/io/skeleton.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -600,12 +600,26 @@ def decode(self, metadata: dict, node_names: list[str]) -> list[Skeleton]:
600600
# New format introduced in SLEAP v1.3.2
601601
# TODO: Do something with the "description" and "preview_image" keys?
602602
skel = skel["nx_graph"]
603+
# Process links with proper py/id resolution.
604+
# In jsonpickle format, py/reduce creates a new object and assigns it
605+
# an implicit py/id (1, 2, 3...). We track which py/id maps to which
606+
# edge type value as we encounter them.
607+
edge_type_map = {} # py/id -> edge_type_value
608+
next_py_id = 1
603609
edge_inds, symmetry_inds = [], []
610+
604611
for link in skel["links"]:
605612
if "py/reduce" in link["type"]:
613+
# New edge type definition - extract value and assign py/id
606614
edge_type = link["type"]["py/reduce"][1]["py/tuple"][0]
607-
else:
608-
edge_type = link["type"]["py/id"]
615+
edge_type_map[next_py_id] = edge_type
616+
next_py_id += 1
617+
elif "py/id" in link["type"]:
618+
# Reference to previously defined edge type - look up the value
619+
py_id = link["type"]["py/id"]
620+
# Fallback to py_id value if not in map (for files where edge types
621+
# are defined in a separate scope or use implicit numbering)
622+
edge_type = edge_type_map.get(py_id, py_id)
609623

610624
if edge_type == 1: # 1 -> real edge, 2 -> symmetry edge
611625
edge_inds.append((link["source"], link["target"]))

tests/io/test_skeleton_io.py

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,3 +1558,112 @@ def test_decode_training_config_invalid_format():
15581558
# Test with non-dict input
15591559
with pytest.raises(ValueError, match="Invalid training config format"):
15601560
decode_training_config("not a dict")
1561+
1562+
1563+
def test_slp_decoder_edge_type_pyid_resolution():
1564+
"""Test that SLP decoder correctly resolves py/id references in edge types.
1565+
1566+
This test catches a bug where py/id values were treated as direct edge type
1567+
values instead of references to previously defined edge types.
1568+
1569+
When a symmetry edge (EdgeType=2) is defined before a regular edge (EdgeType=1):
1570+
- The first py/reduce creates EdgeType(2) and assigns it py/id=1
1571+
- The second py/reduce creates EdgeType(1) and assigns it py/id=2
1572+
- References to py/id=1 should resolve to EdgeType(2), not EdgeType(1)
1573+
- References to py/id=2 should resolve to EdgeType(1), not EdgeType(2)
1574+
1575+
The buggy code treats py/id values as direct edge type values, causing
1576+
edges and symmetries to be swapped.
1577+
"""
1578+
# Create metadata where symmetry (EdgeType=2) is defined before edge (EdgeType=1)
1579+
skeleton_metadata = {
1580+
"directed": True,
1581+
"graph": {"name": "Skeleton-12", "num_edges_inserted": 5},
1582+
"links": [
1583+
# Link 0: Creates EdgeType(2)=SYMMETRY, gets py/id=1
1584+
{
1585+
"key": 0,
1586+
"source": 6,
1587+
"target": 1,
1588+
"type": {
1589+
"py/reduce": [
1590+
{"py/type": "sleap.skeleton.EdgeType"},
1591+
{"py/tuple": [2]},
1592+
]
1593+
},
1594+
},
1595+
# Link 1: References py/id=1 -> should be EdgeType(2)=SYMMETRY
1596+
{"key": 0, "source": 1, "target": 6, "type": {"py/id": 1}},
1597+
# Link 2: Creates EdgeType(1)=EDGE, gets py/id=2
1598+
{
1599+
"edge_insert_idx": 0,
1600+
"key": 0,
1601+
"source": 3,
1602+
"target": 6,
1603+
"type": {
1604+
"py/reduce": [
1605+
{"py/type": "sleap.skeleton.EdgeType"},
1606+
{"py/tuple": [1]},
1607+
]
1608+
},
1609+
},
1610+
# Link 3: References py/id=2 -> should be EdgeType(1)=EDGE
1611+
{
1612+
"edge_insert_idx": 1,
1613+
"key": 0,
1614+
"source": 3,
1615+
"target": 1,
1616+
"type": {"py/id": 2},
1617+
},
1618+
# Link 4: References py/id=2 -> should be EdgeType(1)=EDGE
1619+
{
1620+
"edge_insert_idx": 2,
1621+
"key": 0,
1622+
"source": 3,
1623+
"target": 4,
1624+
"type": {"py/id": 2},
1625+
},
1626+
],
1627+
"multigraph": True,
1628+
"nodes": [{"id": 6}, {"id": 1}, {"id": 3}, {"id": 4}, {"id": 0}],
1629+
}
1630+
1631+
# Create metadata structure as it appears in .slp files
1632+
# The "nodes" list is a global list of all nodes across all skeletons
1633+
metadata = {
1634+
"skeletons": [skeleton_metadata],
1635+
"nodes": [
1636+
{"name": "tailend", "weight": 1.0}, # index 0
1637+
{"name": "right", "weight": 1.0}, # index 1
1638+
{"name": "tailend", "weight": 1.0}, # index 2 (duplicate, not used)
1639+
{"name": "nose", "weight": 1.0}, # index 3
1640+
{"name": "tailstart", "weight": 1.0}, # index 4
1641+
{"name": "tailend", "weight": 1.0}, # index 5 (duplicate, not used)
1642+
{"name": "left", "weight": 1.0}, # index 6
1643+
],
1644+
}
1645+
1646+
# Extract node names the same way as sleap_io.io.slp.read_skeletons()
1647+
node_names = [x["name"] for x in metadata["nodes"]]
1648+
1649+
# Decode using SLP decoder
1650+
decoder = SkeletonSLPDecoder()
1651+
skeletons = decoder.decode(metadata, node_names)
1652+
skeleton = skeletons[0]
1653+
1654+
# Verify skeleton structure
1655+
assert skeleton.name == "Skeleton-12"
1656+
assert len(skeleton.nodes) == 5
1657+
1658+
# Verify edges - should have 3 edges from nose to left/right/tailstart
1659+
assert len(skeleton.edges) == 3
1660+
edge_pairs = [(e.source.name, e.destination.name) for e in skeleton.edges]
1661+
assert ("nose", "left") in edge_pairs
1662+
assert ("nose", "right") in edge_pairs
1663+
assert ("nose", "tailstart") in edge_pairs
1664+
1665+
# Verify symmetries - should have only 1 symmetry between left and right
1666+
# (deduplicated from the two directional links)
1667+
assert len(skeleton.symmetries) == 1
1668+
sym_nodes = {n.name for n in skeleton.symmetries[0].nodes}
1669+
assert sym_nodes == {"left", "right"}

0 commit comments

Comments
 (0)