fix: correct hierarchical position calculation in multi-cycle meters (#36)

- Fix bug where hierarchical position calculation pointed to pulses after query time
- Add pulse time validation and correction logic in get_musical_time()
- Implement _pulse_index_to_hierarchical_position() for reverse calculation
- Add comprehensive test for Issue #36 scenario
- Ensure fractional_beat values are calculated correctly for trajectory analysis

Resolves fractional_beat=0.0 clamping issue that affected musical time analysis
and trajectory curve visualization in multi-cycle meters with timing variations.

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

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

Author: jon-myers

idtap/classes/meter.py

@@ -472,6 +472,36 @@ def _hierarchical_position_to_pulse_index(self, positions: List[int], cycle_numb
         cycle_offset = cycle_number * self._pulses_per_cycle
         return pulse_index + cycle_offset
 
+    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
+        
+        positions = []
+        remaining_index = within_cycle_index
+        
+        # Work from coarsest to finest level
+        for level in range(len(self.hierarchy)):
+            hierarchy_size = self.hierarchy[level]
+            
+            if isinstance(hierarchy_size, list):
+                hierarchy_size = sum(hierarchy_size)
+            
+            # Calculate how many pulses are in each group at this level
+            group_size = self._pulses_per_cycle
+            for inner_level in range(level + 1):
+                inner_size = self.hierarchy[inner_level]
+                if isinstance(inner_size, list):
+                    inner_size = sum(inner_size)
+                group_size = group_size // inner_size
+            
+            position_at_level = remaining_index // group_size
+            positions.append(position_at_level)
+            remaining_index = remaining_index % group_size
+        
+        return positions
+    
     def _calculate_level_start_time(self, positions: List[int], cycle_number: int, reference_level: int) -> float:
         """Calculate start time of hierarchical unit at reference level."""
 
@@ -550,7 +580,7 @@ def get_musical_time(self, real_time: float, reference_level: Optional[int] = No
         total_finest_subdivisions = self._pulses_per_cycle
         current_group_size = total_finest_subdivisions
 
-        for level, size in enumerate(self.hierarchy):
+        for size in self.hierarchy:
             if isinstance(size, list):
                 size = sum(size)
 
@@ -572,12 +602,26 @@ def get_musical_time(self, real_time: float, reference_level: Optional[int] = No
         # This is independent of reference_level, which only affects hierarchical_position truncation
         current_pulse_index = self._hierarchical_position_to_pulse_index(positions, cycle_number)
 
-        # Bounds checking
+        # Bounds checking and hierarchical position correction
         if current_pulse_index < 0 or current_pulse_index >= len(self.all_pulses):
             fractional_beat = 0.0
         else:
             current_pulse_time = self.all_pulses[current_pulse_index].real_time
 
+            # FIX: If the calculated pulse comes after the query time, find the correct pulse
+            # This happens when hierarchical position calculation rounds up due to timing variations
+            if current_pulse_time > real_time:
+                # Find the pulse that comes at or before the query time
+                corrected_pulse_index = current_pulse_index
+                while corrected_pulse_index > 0 and self.all_pulses[corrected_pulse_index].real_time > real_time:
+                    corrected_pulse_index -= 1
+                current_pulse_index = corrected_pulse_index
+                current_pulse_time = self.all_pulses[current_pulse_index].real_time
+                
+                # Update positions to reflect the corrected pulse index
+                # This ensures consistency between hierarchical_position and actual pulse used
+                positions = self._pulse_index_to_hierarchical_position(current_pulse_index, cycle_number)
+            
             # Handle next pulse
             if current_pulse_index + 1 < len(self.all_pulses):
                 next_pulse_time = self.all_pulses[current_pulse_index + 1].real_time

idtap/tests/musical_time_test.py

@@ -805,3 +805,42 @@ def test_deep_investigation_of_fractional_beat_calculation(self):
         print("\nThis test helps identify if the issue is related to position truncation when")
         print("we're in the middle of subdivisions but reference_level=0 calculation starts")
         print("from the wrong subdivision boundary.")
+
+    def test_issue_36_hierarchical_position_correction(self):
+        """Test fix for Issue #36: hierarchical position calculation bug in multi-cycle meters.
+        
+        Ensures that the hierarchical position calculation always finds a pulse that comes
+        at or before the query time, preventing negative fractional_beat values that get
+        clamped to 0.0.
+        """
+        # Create meter with timing variations that expose the hierarchical calculation bug
+        meter = Meter(hierarchy=[4, 4, 2], start_time=4.0, tempo=60.0, repetitions=4)
+        
+        # Introduce pulse timing variations that can trigger the bug
+        for i, pulse in enumerate(meter.all_pulses):
+            if i % 7 == 0:  # Every 7th pulse gets adjusted timing
+                pulse.real_time += 0.05  # 50ms later - enough to cause issues
+        
+        # Re-sort pulses by time after timing modifications
+        meter.pulse_structures[0][0].pulses.sort(key=lambda p: p.real_time)
+        
+        # Test times that would previously cause fractional_beat=0.0 due to the bug
+        test_times = [5.0, 8.5, 12.2, 16.8]
+        
+        for time in test_times:
+            result = meter.get_musical_time(time)
+            
+            # Should be within meter boundaries
+            assert result is not False, f"Time {time} should be within meter boundaries"
+            
+            # The fix ensures fractional_beat is reasonable (not clamped to 0.0 due to bug)
+            assert 0.0 <= result.fractional_beat < 1.0, f"fractional_beat out of range for time {time}"
+            
+            # Verify hierarchical position points to correct pulse
+            pulse_index = meter._hierarchical_position_to_pulse_index(
+                result.hierarchical_position, result.cycle_number
+            )
+            assert 0 <= pulse_index < len(meter.all_pulses), f"Pulse index should be valid for time {time}"
+            
+            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}"