gh-138122: Fix sample counting for filtered profiling modes

The live collector's efficiency bar was incorrectly showing 0% success
rate when using filtered modes like --mode exception where no thread had
an active exception. This happened because samples were only counted as
successful when frames were actually processed, conflating "profiler
health" with "filter hit rate".

Samples are now always counted as successful when the profiler can read
from the target process, regardless of whether any frames matched the
current filter. This ensures the efficiency bar accurately reflects
profiler connectivity rather than filter selectivity. The invariant
total_samples == successful_samples + failed_samples is now properly
maintained.

Author: pablogsal

Lib/profiling/sampling/live_collector/collector.py

@@ -395,11 +395,9 @@ def collect(self, stack_frames):
         if has_gc_frame:
             self.gc_frame_samples += 1
 
-        # Only count as successful if we actually processed frames
-        # This is important for modes like --mode exception where most samples
-        # may be filtered out at the C level
-        if frames_processed:
-            self.successful_samples += 1
+        # Count as successful - the sample worked even if no frames matched the filter
+        # (e.g., in --mode exception when no thread has an active exception)
+        self.successful_samples += 1
         self.total_samples += 1
 
         # Handle input on every sample for instant responsiveness

Lib/profiling/sampling/live_collector/widgets.py

@@ -308,31 +308,21 @@ def draw_sample_stats(self, line, width, elapsed):
 
     def draw_efficiency_bar(self, line, width):
         """Draw sample efficiency bar showing success/failure rates."""
-        success_pct = (
-            self.collector.successful_samples
-            / max(1, self.collector.total_samples)
-        ) * 100
-        failed_pct = (
-            self.collector.failed_samples
-            / max(1, self.collector.total_samples)
-        ) * 100
+        # total_samples = successful_samples + failed_samples, so percentages add to 100%
+        total = max(1, self.collector.total_samples)
+        success_pct = (self.collector.successful_samples / total) * 100
+        failed_pct = (self.collector.failed_samples / total) * 100
 
         col = 0
         self.add_str(line, col, "Efficiency:", curses.A_BOLD)
         col += 11
 
-        label = f" {success_pct:>5.2f}% good, {failed_pct:>4.2f}% failed"
+        label = f" {success_pct:>5.2f}% good, {failed_pct:>5.2f}% failed"
         available_width = width - col - len(label) - 3
 
         if available_width >= MIN_BAR_WIDTH:
             bar_width = min(MAX_EFFICIENCY_BAR_WIDTH, available_width)
-            success_fill = int(
-                (
-                    self.collector.successful_samples
-                    / max(1, self.collector.total_samples)
-                )
-                * bar_width
-            )
+            success_fill = int((self.collector.successful_samples / total) * bar_width)
             failed_fill = bar_width - success_fill
 
             self.add_str(line, col, "[", curses.A_NORMAL)

Lib/test/test_profiling/test_sampling_profiler/test_live_collector_core.py

@@ -267,21 +267,64 @@ def test_collect_with_frames(self):
         self.assertEqual(collector.failed_samples, 0)
 
     def test_collect_with_empty_frames(self):
-        """Test collect with empty frames."""
+        """Test collect with empty frames counts as successful.
+
+        A sample is considered successful if the profiler could read from the
+        target process, even if no frames matched the current filter (e.g.,
+        --mode exception when no thread has an active exception). The sample
+        itself worked; it just didn't produce frame data.
+        """
         collector = LiveStatsCollector(1000)
         thread_info = MockThreadInfo(123, [])
         interpreter_info = MockInterpreterInfo(0, [thread_info])
         stack_frames = [interpreter_info]
 
         collector.collect(stack_frames)
 
-        # Empty frames do NOT count as successful - this is important for
-        # filtered modes like --mode exception where most samples may have
-        # no matching data. Only samples with actual frame data are counted.
-        self.assertEqual(collector.successful_samples, 0)
+        # Empty frames still count as successful - the sample worked even
+        # though no frames matched the filter
+        self.assertEqual(collector.successful_samples, 1)
         self.assertEqual(collector.total_samples, 1)
         self.assertEqual(collector.failed_samples, 0)
 
+    def test_sample_counts_invariant(self):
+        """Test that total_samples == successful_samples + failed_samples.
+
+        This is a regression test ensuring the sample counting invariant holds:
+        - successful_samples: samples where profiler could read process state
+        - failed_samples: samples where reading failed (e.g., process exited)
+        - total_samples: sum of both
+
+        Empty frame data (e.g., from --mode exception with no active exception)
+        still counts as successful - the sample worked, just no frames matched.
+        """
+        collector = LiveStatsCollector(1000)
+
+        # Mix of samples with and without frame data
+        frames = [MockFrameInfo("test.py", 10, "func")]
+        thread_with_frames = MockThreadInfo(123, frames)
+        thread_empty = MockThreadInfo(456, [])
+        interp_with_frames = MockInterpreterInfo(0, [thread_with_frames])
+        interp_empty = MockInterpreterInfo(0, [thread_empty])
+
+        # Collect various samples
+        collector.collect([interp_with_frames])  # Has frames
+        collector.collect([interp_empty])         # No frames (filtered)
+        collector.collect([interp_with_frames])  # Has frames
+        collector.collect([interp_empty])         # No frames (filtered)
+        collector.collect([interp_empty])         # No frames (filtered)
+
+        # All 5 samples are successful (profiler could read process state)
+        self.assertEqual(collector.total_samples, 5)
+        self.assertEqual(collector.successful_samples, 5)
+        self.assertEqual(collector.failed_samples, 0)
+
+        # Invariant must hold
+        self.assertEqual(
+            collector.total_samples,
+            collector.successful_samples + collector.failed_samples
+        )
+
     def test_collect_skip_idle_threads(self):
         """Test that idle threads are skipped when skip_idle=True."""
         collector = LiveStatsCollector(1000, skip_idle=True)
@@ -327,9 +370,10 @@ def test_collect_multiple_threads(self):
     def test_collect_filtered_mode_percentage_calculation(self):
         """Test that percentages use successful_samples, not total_samples.
 
-        This is critical for filtered modes like --mode exception where most
-        samples may be filtered out at the C level. The percentages should
-        be relative to samples that actually had frame data, not all attempts.
+        With the current behavior, all samples are considered successful
+        (the profiler could read from the process), even when filters result
+        in no frame data. This means percentages are relative to all sampling
+        attempts that succeeded in reading process state.
         """
         collector = LiveStatsCollector(1000)
 
@@ -338,35 +382,30 @@ def test_collect_filtered_mode_percentage_calculation(self):
         thread_with_data = MockThreadInfo(123, frames_with_data)
         interpreter_with_data = MockInterpreterInfo(0, [thread_with_data])
 
-        # Empty thread simulates filtered-out data
+        # Empty thread simulates filtered-out data at C level
         thread_empty = MockThreadInfo(456, [])
         interpreter_empty = MockInterpreterInfo(0, [thread_empty])
 
         # 2 samples with data
         collector.collect([interpreter_with_data])
         collector.collect([interpreter_with_data])
 
-        # 8 samples without data (filtered out)
+        # 8 samples without data (filtered out at C level, but sample still succeeded)
         for _ in range(8):
             collector.collect([interpreter_empty])
 
-        # Verify counts
+        # All 10 samples are successful - the profiler could read from the process
         self.assertEqual(collector.total_samples, 10)
-        self.assertEqual(collector.successful_samples, 2)
+        self.assertEqual(collector.successful_samples, 10)
 
         # Build stats and check percentage
         stats_list = collector.build_stats_list()
         self.assertEqual(len(stats_list), 1)
 
-        # The function appeared in 2 out of 2 successful samples = 100%
-        # NOT 2 out of 10 total samples = 20%
+        # The function appeared in 2 out of 10 successful samples = 20%
         location = ("test.py", 10, "exception_handler")
         self.assertEqual(collector.result[location]["direct_calls"], 2)
 
-        # Verify the percentage calculation in build_stats_list
-        # direct_calls / successful_samples * 100 = 2/2 * 100 = 100%
-        # This would be 20% if using total_samples incorrectly
-
     def test_percentage_values_use_successful_samples(self):
         """Test that percentages are calculated from successful_samples.