Merge pull request #726 from lukecotter/fix-incorrect-minimap-bucket-colors

fix: correct minimap bucket category resolution

Author: lcottercertinia

log-viewer/src/features/timeline/__tests__/minimap-density.test.ts

@@ -0,0 +1,198 @@
+/*
+ * Copyright (c) 2026 Certinia Inc. All rights reserved.
+ */
+
+/**
+ * Unit tests for MinimapDensityQuery
+ *
+ * Tests category resolution for minimap coloring, ensuring:
+ * - Long-spanning parent frames are not skipped during frame collection
+ * - Skyline (on-top time) algorithm correctly identifies dominant category
+ * - Both fallback and segment tree paths produce consistent results
+ */
+
+import type { LogEvent } from 'apex-log-parser';
+import { MinimapDensityQuery } from '../optimised/minimap/MinimapDensityQuery.js';
+import type { PrecomputedRect } from '../optimised/RectangleManager.js';
+import { TemporalSegmentTree } from '../optimised/TemporalSegmentTree.js';
+
+/**
+ * Helper to create a mock PrecomputedRect.
+ */
+function createRect(
+  category: string,
+  timeStart: number,
+  timeEnd: number,
+  depth: number,
+  selfDuration?: number,
+): PrecomputedRect {
+  const duration = timeEnd - timeStart;
+  return {
+    id: `${category}-${timeStart}-${depth}`,
+    timeStart,
+    timeEnd,
+    depth,
+    duration,
+    selfDuration: selfDuration ?? duration,
+    category,
+    x: 0,
+    y: 0,
+    width: 0,
+    height: 20,
+    eventRef: { timestamp: timeStart } as LogEvent,
+  };
+}
+
+/**
+ * Build rectsByCategory from a flat list of rects.
+ */
+function buildRectsByCategory(rects: PrecomputedRect[]): Map<string, PrecomputedRect[]> {
+  const map = new Map<string, PrecomputedRect[]>();
+  for (const rect of rects) {
+    let arr = map.get(rect.category);
+    if (!arr) {
+      arr = [];
+      map.set(rect.category, arr);
+    }
+    arr.push(rect);
+  }
+  return map;
+}
+
+describe('MinimapDensityQuery', () => {
+  describe('category resolution with long-spanning parent frames', () => {
+    /**
+     * Regression test: A long parent Method frame spanning many buckets
+     * with a short DML child in the middle.
+     *
+     * The parent Method frame must be included in all overlapping buckets
+     * for correct skyline computation. If frames are collected via binary
+     * search on timeEnd (with timeStart-sorted data), long-spanning parent
+     * frames can be skipped, causing incorrect coloring.
+     *
+     * Layout:
+     *   depth 0: |-------- Method (0-1000) --------|
+     *   depth 1:       |-- DML (300-400) --|
+     *
+     * Expected: Buckets outside DML range should be Method (green).
+     *           Bucket covering DML range should be DML (brown) due to weight.
+     */
+    const rects = [
+      createRect('Method', 0, 1000, 0, 600), // parent, selfDuration excludes DML child time
+      createRect('DML', 300, 400, 1, 100),
+    ];
+
+    it('should show Method in buckets outside DML range (fallback path)', () => {
+      const rectsByCategory = buildRectsByCategory(rects);
+      const query = new MinimapDensityQuery(rectsByCategory, 1000, 1);
+
+      // 10 buckets: each covers 100ns
+      // Bucket 0 [0-100]: only Method → Method
+      // Bucket 3 [300-400]: Method + DML → DML wins (2.5x weight)
+      // Bucket 9 [900-1000]: only Method → Method
+      const result = query.query(10);
+
+      expect(result.buckets[0]!.dominantCategory).toBe('Method');
+      expect(result.buckets[1]!.dominantCategory).toBe('Method');
+      expect(result.buckets[9]!.dominantCategory).toBe('Method');
+
+      // DML bucket: DML at depth 1 is deeper, with 2.5x weight
+      expect(result.buckets[3]!.dominantCategory).toBe('DML');
+    });
+
+    it('should show Method in buckets outside DML range (segment tree path)', () => {
+      const rectsByCategory = buildRectsByCategory(rects);
+      const segmentTree = new TemporalSegmentTree(rectsByCategory);
+      const query = new MinimapDensityQuery(rectsByCategory, 1000, 1, segmentTree);
+
+      const result = query.query(10);
+
+      // These buckets must be Method - the parent frame spans all of them
+      expect(result.buckets[0]!.dominantCategory).toBe('Method');
+      expect(result.buckets[1]!.dominantCategory).toBe('Method');
+      expect(result.buckets[5]!.dominantCategory).toBe('Method');
+      expect(result.buckets[9]!.dominantCategory).toBe('Method');
+
+      // DML bucket
+      expect(result.buckets[3]!.dominantCategory).toBe('DML');
+    });
+
+    it('should produce consistent results between fallback and segment tree paths', () => {
+      const rectsByCategory = buildRectsByCategory(rects);
+      const segmentTree = new TemporalSegmentTree(rectsByCategory);
+
+      const fallbackQuery = new MinimapDensityQuery(rectsByCategory, 1000, 1);
+      const treeQuery = new MinimapDensityQuery(rectsByCategory, 1000, 1, segmentTree);
+
+      const fallbackResult = fallbackQuery.query(10);
+      const treeResult = treeQuery.query(10);
+
+      for (let i = 0; i < 10; i++) {
+        expect(treeResult.buckets[i]!.dominantCategory).toBe(
+          fallbackResult.buckets[i]!.dominantCategory,
+        );
+      }
+    });
+  });
+
+  describe('multiple depth levels with overlapping frames', () => {
+    /**
+     * Layout:
+     *   depth 0: |-------- Code Unit (0-1000) --------|
+     *   depth 1: |-------- Method (0-1000) ------------|
+     *   depth 2:       |-- SOQL (200-300) --|  |-- DML (600-700) --|
+     *
+     * This tests that parent frames at multiple depths are all correctly
+     * collected even when short children exist between them.
+     */
+    it('should resolve Method where no SOQL/DML children exist', () => {
+      const rects = [
+        createRect('Code Unit', 0, 1000, 0, 0), // code unit has 0 self duration (all children)
+        createRect('Method', 0, 1000, 1, 800), // method covers most of the time
+        createRect('SOQL', 200, 300, 2, 100),
+        createRect('DML', 600, 700, 2, 100),
+      ];
+
+      const rectsByCategory = buildRectsByCategory(rects);
+      const segmentTree = new TemporalSegmentTree(rectsByCategory);
+      const query = new MinimapDensityQuery(rectsByCategory, 1000, 2, segmentTree);
+
+      const result = query.query(10);
+
+      // Bucket 0 [0-100]: Code Unit + Method → Method wins (deeper)
+      expect(result.buckets[0]!.dominantCategory).toBe('Method');
+
+      // Bucket 4 [400-500]: Code Unit + Method → Method wins (deeper)
+      expect(result.buckets[4]!.dominantCategory).toBe('Method');
+
+      // Bucket 2 [200-300]: Code Unit + Method + SOQL → SOQL wins (deepest + 2.5x weight)
+      expect(result.buckets[2]!.dominantCategory).toBe('SOQL');
+
+      // Bucket 6 [600-700]: Code Unit + Method + DML → DML wins (deepest + 2.5x weight)
+      expect(result.buckets[6]!.dominantCategory).toBe('DML');
+    });
+  });
+
+  describe('edge cases', () => {
+    it('should handle single frame spanning all buckets', () => {
+      const rects = [createRect('Method', 0, 1000, 0)];
+      const rectsByCategory = buildRectsByCategory(rects);
+      const segmentTree = new TemporalSegmentTree(rectsByCategory);
+      const query = new MinimapDensityQuery(rectsByCategory, 1000, 0, segmentTree);
+
+      const result = query.query(5);
+
+      for (const bucket of result.buckets) {
+        expect(bucket.dominantCategory).toBe('Method');
+      }
+    });
+
+    it('should handle empty timeline', () => {
+      const rectsByCategory = new Map<string, PrecomputedRect[]>();
+      const query = new MinimapDensityQuery(rectsByCategory, 0, 0);
+
+      const result = query.query(10);
+      expect(result.buckets).toHaveLength(0);
+    });
+  });
+});

log-viewer/src/features/timeline/optimised/minimap/MinimapDensityQuery.ts

@@ -332,9 +332,6 @@ export class MinimapDensityQuery {
    * - New: O(N) single pass + O(B × k) skyline computation = ~10-20ms
    *   (where k = avg frames per bucket, much smaller than N)
    *
-   * PERF: Uses binary search to find frames for each bucket on-demand,
-   * avoiding O(N × bucketSpan) frame collection (~10-15ms saved).
-   *
    * @param bucketCount - Number of output buckets
    * @returns MinimapDensityData
    */
@@ -350,16 +347,19 @@ export class MinimapDensityQuery {
 
     const frames = this.segmentTree.getAllFramesSorted();
     const bucketTimeWidth = this.totalDuration / bucketCount;
-    const frameCount = frames.length;
 
     // Pre-allocate bucket arrays
     const maxDepths = new Uint16Array(bucketCount);
     const eventCounts = new Uint32Array(bucketCount);
     const selfDurationSums = new Float64Array(bucketCount);
 
-    // First pass: compute maxDepth, eventCount, and selfDurationSums per bucket
-    // This still needs to iterate frames spanning multiple buckets, but we avoid
-    // storing frame references (which was the main memory/allocation cost)
+    // Collect frames per bucket for skyline computation
+    const framesPerBucket: SkylineFrame[][] = new Array(bucketCount);
+    for (let i = 0; i < bucketCount; i++) {
+      framesPerBucket[i] = [];
+    }
+
+    // Single pass: compute maxDepth, eventCount, selfDurationSums, and collect frames
     for (const frame of frames) {
       const startBucket = Math.max(0, Math.floor(frame.timeStart / bucketTimeWidth));
       const endBucket = Math.min(bucketCount - 1, Math.floor(frame.timeEnd / bucketTimeWidth));
@@ -385,16 +385,16 @@ export class MinimapDensityQuery {
         const overlapRatio = frameDuration > 0 ? visibleTime / frameDuration : 0;
         const proportionalSelfDuration = frame.selfDuration * overlapRatio;
         selfDurationSums[b]! += proportionalSelfDuration;
+
+        // Collect frame for skyline computation
+        framesPerBucket[b]!.push(frame);
       }
     }
 
-    // Build output buckets with on-demand frame lookup for skyline computation
+    // Build output buckets
     const buckets: MinimapDensityBucket[] = new Array(bucketCount);
     let maxEventCount = 0;
 
-    // Track search position for binary search optimization
-    let searchStartIdx = 0;
-
     for (let i = 0; i < bucketCount; i++) {
       const eventCount = eventCounts[i]!;
       if (eventCount > maxEventCount) {
@@ -404,25 +404,9 @@ export class MinimapDensityQuery {
       const bucketStart = i * bucketTimeWidth;
       const bucketEnd = (i + 1) * bucketTimeWidth;
 
-      // PERF: Use binary search to find frames for this bucket on-demand
-      // instead of pre-collecting all frames into all buckets
-      const bucketFrames = this.getFramesInRange(
-        frames,
-        frameCount,
-        bucketStart,
-        bucketEnd,
-        searchStartIdx,
-      );
-
-      // Update search start for next bucket (frames are sorted by timeStart)
-      // Advance searchStartIdx to skip frames that end before this bucket
-      while (searchStartIdx < frameCount && frames[searchStartIdx]!.timeEnd <= bucketStart) {
-        searchStartIdx++;
-      }
-
       // Resolve dominant category using skyline (on-top time) algorithm
       const dominantCategory = this.resolveCategoryFromSkyline(
-        bucketFrames,
+        framesPerBucket[i]!,
         bucketStart,
         bucketEnd,
       );
@@ -445,56 +429,6 @@ export class MinimapDensityQuery {
     };
   }
 
-  /**
-   * Binary search to find frames overlapping a time range.
-   * Uses the fact that frames are sorted by timeStart.
-   *
-   * PERF: O(log N + k) where k = frames in range, vs O(N) for pre-collection.
-   *
-   * @param frames - Sorted frames array
-   * @param frameCount - Number of frames
-   * @param bucketStart - Bucket start time
-   * @param bucketEnd - Bucket end time
-   * @param startIdx - Starting index for search (optimization for sequential buckets)
-   * @returns Frames overlapping the bucket
-   */
-  private getFramesInRange(
-    frames: SkylineFrame[],
-    frameCount: number,
-    bucketStart: number,
-    bucketEnd: number,
-    startIdx: number,
-  ): SkylineFrame[] {
-    // Binary search for first frame that could overlap (timeEnd > bucketStart)
-    let lo = startIdx;
-    let hi = frameCount;
-
-    while (lo < hi) {
-      const mid = (lo + hi) >>> 1;
-      if (frames[mid]!.timeEnd <= bucketStart) {
-        lo = mid + 1;
-      } else {
-        hi = mid;
-      }
-    }
-
-    // Linear scan from found position to collect overlapping frames
-    const result: SkylineFrame[] = [];
-    for (let i = lo; i < frameCount; i++) {
-      const frame = frames[i]!;
-      // Frames are sorted by timeStart, so stop when past bucket
-      if (frame.timeStart >= bucketEnd) {
-        break;
-      }
-      // Check for overlap: frame overlaps bucket if frame.timeEnd > bucketStart
-      if (frame.timeEnd > bucketStart) {
-        result.push(frame);
-      }
-    }
-
-    return result;
-  }
-
   /**
    * Compute density data using per-bucket tree queries.
    * O(B × log N) complexity - for each bucket, query the segment tree.