Fix Issue #38: Resolve cycle boundary failures in get_musical_time() (#39)

* fix: allow exact end time in get_musical_time() boundary validation (#38)

- Change boundary condition from >= to > to allow exact end time
- Add comprehensive test for cycle boundary scenarios
- Update existing boundary test to reflect correct behavior
- Resolves trajectory visualization issues at cycle boundaries

Fixes issue where get_musical_time() returned False for timestamps
at exact cycle boundaries, causing missing trajectory curves in
meter-time visualizations.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: resolve negative hierarchical positions in pulse correction

Fixed _pulse_index_to_hierarchical_position method to prevent negative
positions when correcting pulse indices across cycle boundaries. The method
now uses modulo arithmetic to correctly determine within-cycle positions,
resolving "All hierarchical positions must be non-negative" errors that
occurred with offset pulses from real transcription data.

This completes the comprehensive fix for Issue #38, ensuring get_musical_time()
works correctly with both theoretical and actual pulse timing variations.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: jon-myers

idtap/classes/meter.py

@@ -474,9 +474,12 @@ def _hierarchical_position_to_pulse_index(self, positions: List[int], cycle_numb
 
     def _pulse_index_to_hierarchical_position(self, pulse_index: int, cycle_number: int) -> List[int]:
         """Convert pulse index back to hierarchical position (reverse of _hierarchical_position_to_pulse_index)."""
-        # Remove cycle offset
-        cycle_offset = cycle_number * self._pulses_per_cycle
-        within_cycle_index = pulse_index - cycle_offset
+        # Use modulo to get within-cycle index regardless of which cycle the pulse belongs to
+        within_cycle_index = pulse_index % self._pulses_per_cycle
+        
+        # Ensure within_cycle_index is non-negative
+        if within_cycle_index < 0:
+            within_cycle_index = 0
 
         positions = []
         remaining_index = within_cycle_index
@@ -496,9 +499,9 @@ def _pulse_index_to_hierarchical_position(self, pulse_index: int, cycle_number:
                     inner_size = sum(inner_size)
                 group_size = group_size // inner_size
 
-            position_at_level = remaining_index // group_size
+            position_at_level = remaining_index // group_size if group_size > 0 else 0
             positions.append(position_at_level)
-            remaining_index = remaining_index % group_size
+            remaining_index = remaining_index % group_size if group_size > 0 else 0
 
         return positions
 
@@ -561,8 +564,30 @@ def get_musical_time(self, real_time: float, reference_level: Optional[int] = No
         if real_time < self.start_time:
             return False
 
-        end_time = self.start_time + self.repetitions * self.cycle_dur
-        if real_time >= end_time:
+        # Calculate proper end time based on actual pulse timing
+        # For intermediate cycles: use actual next cycle start pulse
+        # For final cycle: use theoretical calculation (no next cycle exists)
+        if self.all_pulses and len(self.all_pulses) > 0:
+            # Calculate which cycle this time would fall into
+            relative_time = real_time - self.start_time
+            potential_cycle = int(relative_time // self.cycle_dur)
+            
+            if potential_cycle < self.repetitions - 1:
+                # This is an intermediate cycle - use actual next cycle start pulse
+                next_cycle_first_pulse_index = (potential_cycle + 1) * self._pulses_per_cycle
+                if next_cycle_first_pulse_index < len(self.all_pulses):
+                    actual_end_time = self.all_pulses[next_cycle_first_pulse_index].real_time
+                else:
+                    # Fallback to theoretical if pulse doesn't exist
+                    actual_end_time = self.start_time + self.repetitions * self.cycle_dur
+            else:
+                # This is the final cycle - use theoretical end time
+                actual_end_time = self.start_time + self.repetitions * self.cycle_dur
+        else:
+            # No pulses available - use theoretical calculation
+            actual_end_time = self.start_time + self.repetitions * self.cycle_dur
+        
+        if real_time > actual_end_time:
             return False
 
         # Validate reference level

idtap/tests/musical_time_test.py

@@ -164,8 +164,11 @@ def test_boundary_conditions(self):
         result = meter.get_musical_time(end_time - 0.01)
         assert result is not False
 
-        # At or after end
-        assert meter.get_musical_time(end_time) is False
+        # At end (should be valid after Issue #38 fix)
+        result = meter.get_musical_time(end_time)
+        assert result is not False, "Exact end time should be valid (Issue #38 fix)"
+        
+        # After end (should still be invalid)
         assert meter.get_musical_time(end_time + 0.01) is False
 
     def test_reference_level_validation(self):
@@ -844,3 +847,42 @@ def test_issue_36_hierarchical_position_correction(self):
 
             pulse_time = meter.all_pulses[pulse_index].real_time
             assert pulse_time <= time, f"Calculated pulse should come at/before query time {time}, got {pulse_time}"
+
+    def test_issue_38_cycle_boundary_failures(self):
+        """Test fix for Issue #38: get_musical_time() fails at cycle boundaries.
+        
+        Ensures that get_musical_time() returns valid musical time objects for
+        timestamps at exact cycle boundaries, including the final meter boundary.
+        """
+        # Create meter with multiple cycles to test all boundary types
+        meter = Meter(hierarchy=[4, 4, 2], start_time=0.0, tempo=60.0, repetitions=4)
+        
+        # Test each cycle boundary including the final one
+        for cycle in range(meter.repetitions + 1):
+            boundary_time = meter.start_time + cycle * meter.cycle_dur
+            
+            result = meter.get_musical_time(boundary_time)
+            
+            # All boundaries should return valid musical time objects
+            assert result is not False, f"Cycle boundary at {boundary_time} should return valid musical time"
+            assert 0.0 <= result.fractional_beat < 1.0, f"fractional_beat should be in valid range for boundary {boundary_time}"
+            
+            # Boundary should be treated as start of next cycle (if not final boundary)
+            if cycle < meter.repetitions:
+                assert result.cycle_number == cycle, f"Boundary {boundary_time} should be in cycle {cycle}"
+                assert result.hierarchical_position[0] == 0, f"Boundary should be at start of hierarchical position"
+            else:
+                # Final boundary - should be treated as start of theoretical next cycle
+                assert result.cycle_number == cycle, f"Final boundary should indicate cycle {cycle}"
+        
+        # Test times very close to boundaries to ensure they also work
+        for cycle in range(meter.repetitions):
+            boundary_time = meter.start_time + (cycle + 1) * meter.cycle_dur
+            
+            # Test time just before boundary
+            near_boundary = boundary_time - 0.001
+            result = meter.get_musical_time(near_boundary)
+            assert result is not False, f"Time just before boundary {boundary_time} should be valid"
+            
+            # Should be in the previous cycle
+            assert result.cycle_number == cycle, f"Time before boundary should be in cycle {cycle}"