Skip to content

feat: add overview minimap for large dataset navigation#458

Merged
tylerkron merged 13 commits intomainfrom
claude/hungry-mayer
Apr 11, 2026
Merged

feat: add overview minimap for large dataset navigation#458
tylerkron merged 13 commits intomainfrom
claude/hungry-mayer

Conversation

@tylerkron
Copy link
Copy Markdown
Contributor

Summary

  • Adds a minimap control below the logged data plot showing a downsampled overview of the full dataset with a draggable/resizable selection rectangle
  • Min/Max downsampling (800 buckets, ~1600 output points) enables instant rendering even for 10M+ point datasets
  • Two-way synchronization: zooming/panning the main plot updates the minimap indicator, and dragging/resizing the minimap indicator controls the main plot
  • Channel visibility toggles sync between main plot and minimap

Closes #147

New files

  • Daqifi.Desktop/Helpers/MinMaxDownsampler.cs — O(N) single-pass min/max downsampler
  • Daqifi.Desktop/View/MinimapInteractionController.cs — Mouse interaction handler for drag (pan), edge-resize (zoom), and click-to-jump on the minimap

Modified files

  • Daqifi.Desktop/Loggers/DatabaseLogger.cs — Added MinimapPlotModel, downsampled data population, axis change sync, legend visibility sync
  • Daqifi.Desktop/MainWindow.xaml — Added 80px minimap PlotView below the main plot, relocated zoom button overlays into inner grid

Test plan

  • Load a session with data and verify the minimap appears below the main plot with downsampled series
  • Zoom/pan the main plot and verify the selection rectangle updates position/size in real-time
  • Drag the selection rectangle on the minimap and verify the main plot pans accordingly
  • Drag the left/right edges of the selection rectangle and verify the main plot zooms
  • Click outside the rectangle on the minimap and verify it jumps to the click position
  • Reset zoom and verify the rectangle expands to fill the full minimap range
  • Toggle channel visibility in the legend and verify the minimap series visibility matches
  • Load sessions with varying sizes (small, 100K, 1M points) and verify performance
  • Verify zoom/save button overlays remain correctly positioned over the main plot only

🤖 Generated with Claude Code

Add a minimap control below the logged data plot that shows a downsampled
overview of the full dataset with a draggable selection rectangle for
navigating large recordings (10M+ points).

Closes #147

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tylerkron tylerkron requested a review from a team as a code owner April 4, 2026 16:31
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add overview minimap for large dataset navigation

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Adds minimap control below main plot for navigating large datasets
• Implements O(N) min/max downsampling for instant rendering of 10M+ points
• Enables drag/resize/click interactions on minimap selection rectangle
• Synchronizes minimap visibility with main plot zoom and channel toggles
Diagram
flowchart LR
  MainPlot["Main Plot<br/>Full Data Series"]
  Downsampler["MinMaxDownsampler<br/>O(N) Algorithm"]
  Minimap["Minimap PlotModel<br/>Downsampled Overview"]
  Controller["MinimapInteractionController<br/>Mouse Handlers"]
  Sync["Synchronization<br/>Axis & Visibility"]
  
  MainPlot -->|"Downsample 800 buckets"| Downsampler
  Downsampler -->|"~1600 output points"| Minimap
  Minimap -->|"Drag/Resize/Click"| Controller
  Controller -->|"Update zoom range"| MainPlot
  MainPlot -->|"Axis changes"| Sync
  Sync -->|"Update rectangle"| Minimap
Loading

Grey Divider

File Changes

1. Daqifi.Desktop/Helpers/MinMaxDownsampler.cs ✨ Enhancement +96/-0

O(N) min/max downsampling for large datasets

• New static utility class implementing O(N) single-pass min/max downsampling algorithm
• Divides time range into configurable buckets and emits min/max points per bucket
• Preserves visual envelope of data while reducing point count from millions to ~1600
• Handles edge cases like zero range and single-point datasets

Daqifi.Desktop/Helpers/MinMaxDownsampler.cs


2. Daqifi.Desktop/View/MinimapInteractionController.cs ✨ Enhancement +238/-0

Mouse interaction handler for minimap navigation

• New controller class handling mouse interactions on minimap PlotModel
• Supports three drag modes: pan (move rectangle), resize left/right edges, and click-to-jump
• Converts screen coordinates to data coordinates and applies constraints (minimum width, bounds
 clamping)
• Synchronizes minimap selection rectangle changes back to main plot time axis zoom

Daqifi.Desktop/View/MinimapInteractionController.cs


3. Daqifi.Desktop/Loggers/DatabaseLogger.cs ✨ Enhancement +168/-4

Minimap model initialization and synchronization logic

• Added MinimapPlotModel observable property and initialization method
 InitializeMinimapPlotModel()
• Created _minimapSeries dictionary to track downsampled series and _minimapSelectionRect
 annotation
• Integrated MinimapInteractionController to handle user interactions on minimap
• Added OnMainTimeAxisChanged event handler to sync main plot axis changes to minimap rectangle
• Implemented SetMinimapSeriesVisibility() method to toggle minimap series when legend items are
 toggled
• Modified LoggedSeriesLegendItem to accept DatabaseLogger reference for visibility sync
 callback
• Updated DisplayLoggingSession() to downsample all session data and populate minimap with colored
 series
• Updated ClearPlot() to clear minimap series and reset minimap plot model

Daqifi.Desktop/Loggers/DatabaseLogger.cs


View more (1)
4. Daqifi.Desktop/MainWindow.xaml ✨ Enhancement +51/-35

Add minimap PlotView and reorganize button overlays

• Restructured main plot grid to add nested row definitions (2* for main plot, * for minimap)
• Moved save/zoom button overlays from floating DockPanels into Grid.Row="0" to stay above main plot
 only
• Added new PlotView for minimap in Grid.Row="1" with 80px height and binding to
 DbLogger.MinimapPlotModel
• Maintained legend column layout and button styling without functional changes

Daqifi.Desktop/MainWindow.xaml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Apr 4, 2026

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (1)   📎 Requirement gaps (0)   🎨 UX Issues (0)
🐞\ ≡ Correctness (1)
📘\ ☼ Reliability (1)

Grey Divider


Action required

1. MinimapPlotModel missing XML docs📘
Description
A new public API surface (MinimapPlotModel generated by [ObservableProperty]) is introduced
without XML documentation comments. This violates the requirement that newly added/modified public
members be self-documented for maintainability and correct usage.
Code

Daqifi.Desktop/Loggers/DatabaseLogger.cs[R96-100]

    [ObservableProperty]
    private PlotModel _plotModel;
+
+    [ObservableProperty]
+    private PlotModel _minimapPlotModel;
Evidence
PR Compliance ID 32 requires XML documentation on new/modified public API members; the new
[ObservableProperty] backing field _minimapPlotModel introduces a public MinimapPlotModel
property without any XML doc comment adjacent to the added member.

Daqifi.Desktop/Loggers/DatabaseLogger.cs[96-100]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new public property (`MinimapPlotModel`) is introduced via `[ObservableProperty]` without XML documentation comments.

## Issue Context
The project requires XML docs on newly added/modified public API members.

## Fix Focus Areas
- Daqifi.Desktop/Loggers/DatabaseLogger.cs[96-100]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Minimap events never unsubscribed 📘
Description
New event subscriptions (timeAxis.AxisChanged and minimap mouse events) are added without any
deterministic teardown/unsubscription path, risking leaks and duplicate callbacks if plot
models/loggers are recreated. This violates the requirement for deterministic event handler
teardown.
Code

Daqifi.Desktop/Loggers/DatabaseLogger.cs[R172-176]

+        // Subscribe to main time axis changes for minimap sync
+        timeAxis.AxisChanged += OnMainTimeAxisChanged;
+
+        // Initialize minimap PlotModel
+        InitializeMinimapPlotModel();
Evidence
PR Compliance ID 35 requires unsubscribing event handlers during teardown; the PR adds
timeAxis.AxisChanged += OnMainTimeAxisChanged and also adds a controller that subscribes to
minimap mouse events, but no corresponding teardown/unsubscribe mechanism is introduced alongside
these additions.

Daqifi.Desktop/Loggers/DatabaseLogger.cs[172-176]
Daqifi.Desktop/View/MinimapInteractionController.cs[43-46]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New event handlers are subscribed (`AxisChanged`, `MouseDown/Move/Up`) without a deterministic teardown path, which can lead to leaks and duplicate callbacks.

## Issue Context
Event handler subscriptions should be unsubscribed during teardown/disposal, especially when objects may be re-instantiated or when views are closed.

## Fix Focus Areas
- Daqifi.Desktop/Loggers/DatabaseLogger.cs[172-176]
- Daqifi.Desktop/View/MinimapInteractionController.cs[43-46]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Constructor line exceeds 120📘
Description
The modified LoggedSeriesLegendItem(...) constructor signature is a single long line that exceeds
the 120 character limit. This violates the repository formatting/layout standards and increases diff
readability issues.
Code

Daqifi.Desktop/Loggers/DatabaseLogger.cs[62]

+    public LoggedSeriesLegendItem(string displayName, string channelName, string deviceSerialNo, OxyColor seriesColor, bool isVisible, LineSeries actualSeries, PlotModel plotModel, DatabaseLogger databaseLogger = null)
Evidence
PR Compliance ID 11 requires a maximum line length of 120 characters; the constructor signature is
written on one line with many parameters, exceeding the limit.

CLAUDE.md
Daqifi.Desktop/Loggers/DatabaseLogger.cs[62-62]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A modified constructor signature exceeds the repository's 120 character line length limit.

## Issue Context
Repository formatting rules require wrapping long method/constructor signatures into multi-line parameter lists.

## Fix Focus Areas
- Daqifi.Desktop/Loggers/DatabaseLogger.cs[62-62]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (2)
4. LoggedSeriesLegendItem ctor undocumented📘
Description
The modified public constructor LoggedSeriesLegendItem(...) has no XML documentation comments
despite being a public API entry point. This violates the public API documentation requirement and
makes the new databaseLogger parameter easier to misuse.
Code

Daqifi.Desktop/Loggers/DatabaseLogger.cs[R62-65]

+    public LoggedSeriesLegendItem(string displayName, string channelName, string deviceSerialNo, OxyColor seriesColor, bool isVisible, LineSeries actualSeries, PlotModel plotModel, DatabaseLogger databaseLogger = null)
    {
        _displayName = displayName;
        _channelName = channelName;
Evidence
PR Compliance ID 32 requires XML docs on modified public API members; the constructor signature was
modified to add DatabaseLogger databaseLogger = null but no XML doc comment was added for the
constructor or its parameters.

Daqifi.Desktop/Loggers/DatabaseLogger.cs[62-73]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The modified public constructor for `LoggedSeriesLegendItem` lacks XML documentation, including for the newly added `databaseLogger` parameter.

## Issue Context
Public API members must be documented with XML comments to support IntelliSense and maintainability.

## Fix Focus Areas
- Daqifi.Desktop/Loggers/DatabaseLogger.cs[62-73]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Unordered data breaks minimap🐞
Description
DisplayLoggingSession populates _allSessionPoints from an EF query without ordering, but
MinMaxDownsampler assumes time-sorted points and uses first/last X to compute xRange; if DB returns
rows out-of-order the minimap can collapse to a single point or produce incorrect bucket
aggregation. This can also make _firstTime come from a non-earliest sample, producing
negative/incorrect deltaTime values downstream.
Code

Daqifi.Desktop/Loggers/DatabaseLogger.cs[R393-406]

+            // Prepare downsampled minimap data on the background thread
+            var minimapSeriesData = new List<(string channelName, string deviceSerial, OxyColor color, List<DataPoint> downsampled)>();
+            foreach (var kvp in _allSessionPoints)
+            {
+                if (kvp.Value.Count > 0)
+                {
+                    var downsampled = MinMaxDownsampler.Downsample(kvp.Value, MINIMAP_BUCKET_COUNT);
+                    var matchingSeries = tempSeriesList.FirstOrDefault(s =>
+                    {
+                        var parts = s.Title.Split([" : ("], StringSplitOptions.None);
+                        return parts.Length == 2 && parts[0] == kvp.Key.channelName && parts[1].TrimEnd(')') == kvp.Key.deviceSerial;
+                    });
+                    minimapSeriesData.Add((kvp.Key.channelName, kvp.Key.deviceSerial, matchingSeries?.Color ?? OxyColors.Gray, downsampled));
+                }
Evidence
DatabaseLogger queries Samples without OrderBy before computing deltaTime and building per-channel
point lists, and then passes those lists to MinMaxDownsampler which explicitly expects time-sorted
data and derives xMin/xMax from first/last elements (making it sensitive to ordering).

Daqifi.Desktop/Loggers/DatabaseLogger.cs[345-390]
Daqifi.Desktop/Loggers/DatabaseLogger.cs[393-407]
Daqifi.Desktop/Helpers/MinMaxDownsampler.cs[10-36]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`MinMaxDownsampler` assumes `points` are time-sorted (`points[0]` is min X and `points[^1]` is max X). In `DisplayLoggingSession`, `dbSamples` is materialized without an `OrderBy`, so `_allSessionPoints` can be out-of-order, producing wrong `deltaTime` and a broken minimap (including the `xRange <= 0` fast-path returning a single point).

### Issue Context
EF Core does not guarantee row order without an explicit `OrderBy`. The minimap logic depends on sorted X.

### Fix Focus Areas
- Daqifi.Desktop/Loggers/DatabaseLogger.cs[345-390]
- Daqifi.Desktop/Loggers/DatabaseLogger.cs[393-407]
- Daqifi.Desktop/Helpers/MinMaxDownsampler.cs[26-35]

### Suggested fix
- Add `.OrderBy(s => s.TimestampTicks)` to the `dbSamples` query.
- (Optional hardening) If you cannot order at the query level, sort each channel list by `X` before downsampling, or adjust `Downsample` to compute true min/max X rather than assuming endpoints.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

6. Extra class in DatabaseLogger.cs📘
Description
DeviceLegendGroup was added as a separate public type inside DatabaseLogger.cs, increasing the
number of top-level classes in a single file. This violates the repository code layout guideline of
keeping one main class per file, reducing maintainability and reviewability.
Code

Daqifi.Desktop/Loggers/DatabaseLogger.cs[R26-53]

+/// <summary>
+/// Groups legend items by device for compact display in the legend panel.
+/// </summary>
+public partial class DeviceLegendGroup : ObservableObject
+{
+    /// <summary>
+    /// Full device serial number.
+    /// </summary>
+    public string DeviceSerialNo { get; }
+
+    /// <summary>
+    /// Truncated serial for display (e.g., "...4104").
+    /// </summary>
+    public string TruncatedSerialNo => DeviceSerialNo?.Length > 4
+        ? $"...{DeviceSerialNo[^4..]}"
+        : DeviceSerialNo ?? string.Empty;
+
+    /// <summary>
+    /// Channel legend items belonging to this device.
+    /// </summary>
+    public ObservableCollection<LoggedSeriesLegendItem> Channels { get; } = new();
+
+    public DeviceLegendGroup(string deviceSerialNo)
+    {
+        DeviceSerialNo = deviceSerialNo;
+    }
+}
+
Evidence
The style checklist requires one main class per file; the new DeviceLegendGroup type is introduced
as an additional top-level class in DatabaseLogger.cs, which already contains other main classes
(LoggedSeriesLegendItem, DatabaseLogger).

CLAUDE.md
Daqifi.Desktop/Loggers/DatabaseLogger.cs[26-53]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DeviceLegendGroup` is a top-level public class defined in `DatabaseLogger.cs`, which now contains multiple main classes. This conflicts with the repo's layout/style expectation of one main class per file.

## Issue Context
Keeping top-level types in separate files improves discoverability and reduces merge conflicts/style-only diffs.

## Fix Focus Areas
- Daqifi.Desktop/Loggers/DatabaseLogger.cs[26-53]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Minimap dim range clipped 🐞
Description
In DisplayLoggingSession, the minimap dim overlays are initialized using the first/last X values of
the downsampled series, but min/max downsampling does not guarantee the true first/last timestamps
are present in the output. This can cause the minimap to dim the start/end of the dataset
incorrectly on initial load (cosmetic inconsistency until the main axis changes).
Code

Daqifi.Desktop/Loggers/DatabaseLogger.cs[R584-585]

+                    _minimapDimLeft.MaximumX = dataMinX;
+                    _minimapDimRight.MinimumX = dataMaxX;
Evidence
The dim overlays are set from dataMinX/dataMaxX derived from downsampled points, and the
downsampler only emits per-bucket extrema (min/max), which can omit the original first/last time
points if they are not extrema in their buckets.

Daqifi.Desktop/Loggers/DatabaseLogger.cs[576-586]
Daqifi.Desktop/Helpers/MinMaxDownsampler.cs[17-92]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The minimap dim overlays (`_minimapDimLeft` / `_minimapDimRight`) are initialized from `dataMinX`/`dataMaxX` computed off the downsampled series. Min/max downsampling can omit the actual first/last timestamps, so the dim overlays may be positioned inward from the true dataset bounds on initial load.

### Issue Context
`MinMaxDownsampler.Downsample` emits only min/max points per bucket and does not guarantee inclusion of the original first/last points.

### Fix Focus Areas
- Daqifi.Desktop/Loggers/DatabaseLogger.cs[576-586]

### Suggested change
Compute initial dim bounds from the true full-resolution time bounds (O(1) per channel):
- Use `_allSessionPoints` values: `Min(v => v[0].X)` and `Max(v => v[^1].X)` (lists are time-ordered due to the query ordering).
- Alternatively, keep `dataMinX/dataMaxX` for the selection rect if you want it based on rendered minimap data, but set dim overlays to the true bounds so the ends never appear incorrectly dimmed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. SetCursor uses Dispatcher.Invoke📘
Description
MinimapInteractionController.SetCursor uses synchronous Dispatcher.Invoke, and it is called
during mouse move/interaction, which can introduce UI-thread blocking and degrade interaction
responsiveness. Prefer non-blocking cursor updates (direct set when on UI thread, otherwise
BeginInvoke) to meet UI responsiveness requirements.
Code

Daqifi.Desktop/View/MinimapInteractionController.cs[R89-94]

+    private void SetCursor(Cursor cursor)
+    {
+        if (_minimapPlotModel.PlotView is FrameworkElement element)
+        {
+            Application.Current?.Dispatcher.Invoke(() => element.Cursor = cursor);
+        }
Evidence
The compliance checklist requires avoiding UI thread blocking; the new SetCursor implementation
performs a synchronous dispatcher call (Dispatcher.Invoke) which can block during high-frequency
input events like mouse move.

CLAUDE.md
Daqifi.Desktop/View/MinimapInteractionController.cs[89-94]
Daqifi.Desktop/View/MinimapInteractionController.cs[169-175]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SetCursor` uses `Application.Current?.Dispatcher.Invoke(...)`, which is synchronous and can block during frequent mouse interactions.

## Issue Context
This controller updates the cursor in response to minimap mouse events (including hover/mouse move). Using synchronous dispatch can introduce noticeable stalls if the call crosses threads or if the dispatcher is busy.

## Fix Focus Areas
- Daqifi.Desktop/View/MinimapInteractionController.cs[89-95]
- Daqifi.Desktop/View/MinimapInteractionController.cs[169-176]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (2)
9. Invalid minimap range math🐞
Description
MinimapInteractionController computes fullRange as axis.DataMaximum - axis.DataMinimum and uses it
to clamp/resize and compute minWidth, without handling NaN/Infinity or zero/negative ranges. This
can propagate invalid values into selection rectangle bounds and then into
mainTimeAxis.Zoom(min,max), breaking zoom/pan interaction.
Code

Daqifi.Desktop/View/MinimapInteractionController.cs[R221-236]

+    private double GetMinimapDataRange()
+    {
+        var axis = GetMinimapTimeAxis();
+        if (axis == null)
+        {
+            return 1;
+        }
+
+        return axis.DataMaximum - axis.DataMinimum;
+    }
+
+    private double GetMinimapDataMin()
+    {
+        var axis = GetMinimapTimeAxis();
+        return axis?.DataMinimum ?? 0;
+    }
Evidence
GetMinimapDataRange returns a raw subtraction and callers use it for edge tolerance, minWidth, and
clamp bounds; ApplyToMainPlot then zooms the main axis using these derived values without validating
min<max or finiteness.

Daqifi.Desktop/View/MinimapInteractionController.cs[63-97]
Daqifi.Desktop/View/MinimapInteractionController.cs[119-186]
Daqifi.Desktop/View/MinimapInteractionController.cs[203-236]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Minimap interaction math assumes `DataMaximum > DataMinimum` and both are finite. When that’s not true, range-based computations can become NaN/0 and feed invalid values into `_selectionRect` and `mainTimeAxis.Zoom(min,max)`.

### Issue Context
This can occur when the minimap has no data yet, when all series are invisible, or before axes have computed data bounds.

### Fix Focus Areas
- Daqifi.Desktop/View/MinimapInteractionController.cs[63-97]
- Daqifi.Desktop/View/MinimapInteractionController.cs[119-186]
- Daqifi.Desktop/View/MinimapInteractionController.cs[203-236]

### Suggested fix
- In `GetMinimapDataRange()`, return a safe positive default when:
 - `!double.IsFinite(axis.DataMinimum/DataMaximum)`
 - `axis.DataMaximum <= axis.DataMinimum`
- In `ApplyToMainPlot(min,max)`, early-return unless `double.IsFinite(min/max)` and `max > min`.
- Consider disabling interactions when range is invalid (e.g., `_dragMode = None`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


10. Downsampler lacks input guards🐞
Description
MinMaxDownsampler.Downsample will throw on empty input (points[0]) and can divide by zero when
bucketCount is 0 (xRange / bucketCount). As a public helper, it should defensively validate inputs
to avoid hard-to-diagnose runtime crashes if reused or called with unexpected parameters.
Code

Daqifi.Desktop/Helpers/MinMaxDownsampler.cs[R17-36]

+    public static List<DataPoint> Downsample(IReadOnlyList<DataPoint> points, int bucketCount)
+    {
+        if (points.Count <= bucketCount * 2)
+        {
+            return new List<DataPoint>(points);
+        }
+
+        var result = new List<DataPoint>(bucketCount * 2);
+
+        var xMin = points[0].X;
+        var xMax = points[points.Count - 1].X;
+        var xRange = xMax - xMin;
+
+        if (xRange <= 0)
+        {
+            return [points[0]];
+        }
+
+        var bucketWidth = xRange / bucketCount;
+        var pointIndex = 0;
Evidence
The implementation indexes points[0]/points[points.Count-1] without checking points.Count > 0
and divides by bucketCount without validating it is > 0.

Daqifi.Desktop/Helpers/MinMaxDownsampler.cs[17-36]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`MinMaxDownsampler.Downsample` can throw `IndexOutOfRangeException` for `points.Count == 0` and `DivideByZeroException` for `bucketCount == 0`.

### Issue Context
Even if the current caller uses non-empty lists and a constant bucket count, this is a reusable helper and should fail fast with clear exceptions (or return an empty list) on invalid inputs.

### Fix Focus Areas
- Daqifi.Desktop/Helpers/MinMaxDownsampler.cs[17-36]

### Suggested fix
- Add guards at the top:
 - `if (points is null) throw new ArgumentNullException(nameof(points));`
 - `if (bucketCount <= 0) throw new ArgumentOutOfRangeException(nameof(bucketCount));`
 - `if (points.Count == 0) return new List<DataPoint>();`
- Consider documenting the required sort order and/or enforcing it (e.g., optional assert in Debug builds).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

11. Redundant cursor updates🐞
Description
MinimapInteractionController updates the WPF cursor on every hover mouse-move even when the cursor
value hasn’t changed. This creates avoidable per-event work in a high-frequency path (mouse move)
and can be reduced by caching the last cursor and only applying changes when it differs.
Code

Daqifi.Desktop/View/MinimapInteractionController.cs[R89-95]

+    private void SetCursor(Cursor cursor)
+    {
+        if (_minimapPlotModel.PlotView is FrameworkElement element)
+        {
+            Application.Current?.Dispatcher.Invoke(() => element.Cursor = cursor);
+        }
+    }
Evidence
On hover (_dragMode == None), OnMouseMove calls SetCursor(GetCursorForPosition(dataX)) for
every mouse move; SetCursor assigns the cursor via dispatcher each time without checking whether
the cursor already matches.

Daqifi.Desktop/View/MinimapInteractionController.cs[89-95]
Daqifi.Desktop/View/MinimapInteractionController.cs[161-176]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The minimap updates the cursor on every hover mouse move even when the cursor does not change. This is unnecessary work in a very hot path.

### Issue Context
`OnMouseMove` runs at high frequency. Cursor updates can be reduced by caching the last applied cursor and only setting when it changes.

### Fix Focus Areas
- Daqifi.Desktop/View/MinimapInteractionController.cs[61-97]
- Daqifi.Desktop/View/MinimapInteractionController.cs[169-176]

### Suggested change
- Add a private field like `_lastCursor`.
- In `OnMouseMove` (hover case), compute `newCursor` and only call `SetCursor(newCursor)` when it differs from `_lastCursor`.
- Optionally, in `SetCursor`, use `element.Dispatcher` (instead of `Application.Current.Dispatcher`) and set directly when `CheckAccess()` is true.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread Daqifi.Desktop/Loggers/DatabaseLogger.cs
Comment thread Daqifi.Desktop/Loggers/DatabaseLogger.cs Outdated
Comment on lines +172 to +176
// Subscribe to main time axis changes for minimap sync
timeAxis.AxisChanged += OnMainTimeAxisChanged;

// Initialize minimap PlotModel
InitializeMinimapPlotModel();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

3. Minimap events never unsubscribed 📘 Rule violation ☼ Reliability

New event subscriptions (timeAxis.AxisChanged and minimap mouse events) are added without any
deterministic teardown/unsubscription path, risking leaks and duplicate callbacks if plot
models/loggers are recreated. This violates the requirement for deterministic event handler
teardown.
Agent Prompt
## Issue description
New event handlers are subscribed (`AxisChanged`, `MouseDown/Move/Up`) without a deterministic teardown path, which can lead to leaks and duplicate callbacks.

## Issue Context
Event handler subscriptions should be unsubscribed during teardown/disposal, especially when objects may be re-instantiated or when views are closed.

## Fix Focus Areas
- Daqifi.Desktop/Loggers/DatabaseLogger.cs[172-176]
- Daqifi.Desktop/View/MinimapInteractionController.cs[43-46]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. MinimapInteractionController now implements IDisposable and unsubscribes all mouse event handlers in Dispose(). Note: the axis event and controller are created once in the DatabaseLogger constructor and live for the app lifetime (single instance), so there's no practical leak risk — but the teardown path is now available. The pre-existing CA1001 warning on DatabaseLogger itself (for _buffer and _consumerGate) is a separate issue tracked outside this PR.

Comment thread Daqifi.Desktop/Loggers/DatabaseLogger.cs Outdated
Comment thread Daqifi.Desktop/Loggers/DatabaseLogger.cs
- Add XML docs on MinimapPlotModel and LoggedSeriesLegendItem constructor
- Wrap constructor signature to respect 120-char line limit
- Add OrderBy(TimestampTicks) to ensure time-sorted data for downsampling

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tylerkron and others added 9 commits April 10, 2026 22:09
Replace the faint selection rectangle with two semi-transparent white
overlays that dim the non-selected areas, making the viewed region
clearly stand out. The selection rectangle border remains for precise
edge identification during drag/resize interactions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
White semi-transparent overlays are invisible on a white background.
Switch to grey (200,200,200) at 59% opacity so non-selected areas
are visibly dimmed. Also replace double.MinValue/MaxValue with 1e18
to avoid potential OxyPlot rendering issues with extreme coordinates.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n border

- Remove Y-axis labels/ticks and X-axis tick labels from minimap
- Reduce plot margins to use full width without label padding
- Increase selection rectangle stroke to 3px with darker blue

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Truncate device serial numbers to "...XXXX" (last 4 digits) in legend
- Full serial number shown in tooltip on hover
- Add chevron toggle button to collapse/expand the legend panel
- Reduce legend width (120-160px) and item padding for compact layout

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Group channels by device with a single device header showing
  truncated serial ("...4104"), full serial in tooltip
- Display channels in a compact 2-column UniformGrid (color swatch +
  name only, no repeated serial per channel)
- Each channel remains individually toggleable for visibility
- Legend still collapsible via chevron button
- Significantly reduces legend width and vertical space usage

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- SizeWE (horizontal resize arrows) when hovering over left/right
  edges of the selection rectangle
- Hand cursor when hovering inside the selection rectangle (pan)
- Arrow cursor when hovering outside the selection
- ScrollAll cursor while actively dragging/panning
- Cursors update on hover, drag start, and drag end

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Hide the main plot, minimap, zoom controls, and legend when no
  session data is loaded
- Show a centered placeholder with chart icon and "Select a session
  to view data" message in the empty state
- Minimap row uses Auto height so it collapses completely when hidden

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tylerkron
Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Apr 11, 2026

Persistent review updated to latest commit 1369898

tylerkron and others added 2 commits April 11, 2026 12:31
- Add XML param docs to LoggedSeriesLegendItem constructor
- Implement IDisposable on MinimapInteractionController to unsubscribe
  mouse event handlers during teardown

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move DeviceLegendGroup to its own file (one class per file rule)
- Switch SetCursor from Dispatcher.Invoke to BeginInvoke for non-blocking UI
- Cache last cursor to avoid redundant updates on mouse move
- Add null/empty/zero input guards to MinMaxDownsampler.Downsample
- Guard GetMinimapDataRange against NaN/Infinity/zero returns

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tylerkron
Copy link
Copy Markdown
Contributor Author

Addressed all remaining Qodo summary items (#6#11) in commit b46e6bc:

#6 — Extra class in DatabaseLogger.cs: Fixed. Moved DeviceLegendGroup to its own file Daqifi.Desktop/Loggers/DeviceLegendGroup.cs.

#7 — SetCursor uses Dispatcher.Invoke: Fixed. Switched to Dispatcher.BeginInvoke for non-blocking cursor updates.

#8 — Minimap dim range clipped: Disagree. The dataMinX/dataMaxX values are computed from the raw _allSessionPoints data (not the downsampled output), so they represent the true data bounds. The dim overlays are initialized to match the selection rectangle which spans the full data range on initial load, so there's no cosmetic inconsistency.

#9 — Downsampler lacks input guards: Fixed. Added ArgumentNullException.ThrowIfNull, empty list check, and bucketCount <= 0 guard.

#10 — Invalid minimap range math: Fixed. GetMinimapDataRange() now guards against NaN, Infinity, and <= 0 returns, falling back to 1.

#11 — Redundant cursor updates: Fixed. Added _lastCursor cache — SetCursor now skips the dispatcher call if the cursor value hasn't changed.

@github-actions
Copy link
Copy Markdown

📊 Code Coverage Report

Summary

Summary
Generated on: 4/11/2026 - 6:37:16 PM
Coverage date: 4/11/2026 - 6:36:47 PM - 4/11/2026 - 6:37:12 PM
Parser: MultiReport (4x Cobertura)
Assemblies: 3
Classes: 113
Files: 144
Line coverage: 18.8% (1227 of 6525)
Covered lines: 1227
Uncovered lines: 5298
Coverable lines: 6525
Total lines: 19505
Branch coverage: 16.9% (404 of 2389)
Covered branches: 404
Total branches: 2389
Method coverage: Feature is only available for sponsors

Coverage

DAQiFi - 18.5%
Name Line Branch
DAQiFi 18.5% 16.9%
Daqifi.Desktop.App 6.2% 0%
Daqifi.Desktop.Channel.AbstractChannel 40.9% 27.7%
Daqifi.Desktop.Channel.AnalogChannel 58.7% 25%
Daqifi.Desktop.Channel.Channel 11.5% 0%
Daqifi.Desktop.Channel.ChannelColorManager 100% 100%
Daqifi.Desktop.Channel.DataSample 91.6%
Daqifi.Desktop.Channel.DigitalChannel 65.2% 25%
Daqifi.Desktop.Commands.CompositeCommand 0% 0%
Daqifi.Desktop.Commands.HostCommands 0%
Daqifi.Desktop.Commands.WeakEventHandlerManager 0% 0%
Daqifi.Desktop.Configuration.FirewallConfiguration 90.6% 66.6%
Daqifi.Desktop.Configuration.WindowsFirewallWrapper 64% 68.4%
Daqifi.Desktop.ConnectionManager 42.4% 39.2%
Daqifi.Desktop.Converters.BoolToActiveStatusConverter 0% 0%
Daqifi.Desktop.Converters.BoolToConnectionStatusConverter 0% 0%
Daqifi.Desktop.Converters.BoolToStatusColorConverter 0% 0%
Daqifi.Desktop.Converters.ConnectionTypeToColorConverter 0% 0%
Daqifi.Desktop.Converters.ConnectionTypeToUsbConverter 0% 0%
Daqifi.Desktop.Converters.InvertedBoolToVisibilityConverter 0% 0%
Daqifi.Desktop.Converters.ListToStringConverter 0% 0%
Daqifi.Desktop.Converters.NotNullToVisibilityConverter 0% 0%
Daqifi.Desktop.Converters.OxyColorToBrushConverter 0% 0%
Daqifi.Desktop.Converters.StringRightConverter 0% 0%
Daqifi.Desktop.Device.AbstractStreamingDevice 42.9% 38.6%
Daqifi.Desktop.Device.DeviceMessage 0%
Daqifi.Desktop.Device.Firmware.BootloaderSessionStreamingDeviceAdapter 0% 0%
Daqifi.Desktop.Device.Firmware.WifiPromptDelayProcessRunner 0% 0%
Daqifi.Desktop.Device.NativeMethods 100%
Daqifi.Desktop.Device.SerialDevice.SerialStreamingDevice 27.6% 30.8%
Daqifi.Desktop.Device.WiFiDevice.DaqifiStreamingDevice 40.9% 39.4%
Daqifi.Desktop.DialogService.DialogService 0% 0%
Daqifi.Desktop.DialogService.ServiceLocator 0% 0%
Daqifi.Desktop.DiskSpace.DiskSpaceCheckResult 100%
Daqifi.Desktop.DiskSpace.DiskSpaceEventArgs 100%
Daqifi.Desktop.DiskSpace.DiskSpaceMonitor 88.2% 86.6%
Daqifi.Desktop.DuplicateDeviceCheckResult 100%
Daqifi.Desktop.Exporter.OptimizedLoggingSessionExporter 30.2% 35.4%
Daqifi.Desktop.Exporter.SampleData 0%
Daqifi.Desktop.Helpers.BooleanConverter`1 0% 0%
Daqifi.Desktop.Helpers.BooleanToInverseBoolConverter 0% 0%
Daqifi.Desktop.Helpers.BooleanToVisibilityConverter 0%
Daqifi.Desktop.Helpers.EnumDescriptionConverter 100% 100%
Daqifi.Desktop.Helpers.IntToVisibilityConverter 0% 0%
Daqifi.Desktop.Helpers.MinMaxDownsampler 0% 0%
Daqifi.Desktop.Helpers.MyMultiValueConverter 0%
Daqifi.Desktop.Helpers.NaturalSortHelper 100% 100%
Daqifi.Desktop.Helpers.VersionHelper 98.2% 66.2%
Daqifi.Desktop.Logger.DatabaseLogger 0% 0%
Daqifi.Desktop.Logger.DeviceLegendGroup 0% 0%
Daqifi.Desktop.Logger.LoggedSeriesLegendItem 0% 0%
Daqifi.Desktop.Logger.LoggingContext 0%
Daqifi.Desktop.Logger.LoggingManager 0% 0%
Daqifi.Desktop.Logger.LoggingSession 42.8% 50%
Daqifi.Desktop.Logger.PlotLogger 0% 0%
Daqifi.Desktop.Logger.SummaryLogger 0% 0%
Daqifi.Desktop.Logger.TimestampGapDetector 95% 83.3%
Daqifi.Desktop.Loggers.ImportOptions 0%
Daqifi.Desktop.Loggers.ImportProgress 0% 0%
Daqifi.Desktop.Loggers.SdCardSessionImporter 0% 0%
Daqifi.Desktop.MainWindow 0% 0%
Daqifi.Desktop.Migrations.InitialSQLiteMigration 0%
Daqifi.Desktop.Migrations.LoggingContextModelSnapshot 0%
Daqifi.Desktop.Models.AddProfileModel 0%
Daqifi.Desktop.Models.DaqifiSettings 80.5% 83.3%
Daqifi.Desktop.Models.DebugDataCollection 6.6% 0%
Daqifi.Desktop.Models.DebugDataModel 0% 0%
Daqifi.Desktop.Models.Notifications 0%
Daqifi.Desktop.Models.SdCardFile 0% 0%
Daqifi.Desktop.Services.WindowsPrincipalAdminChecker 0%
Daqifi.Desktop.Services.WpfMessageBoxService 0%
Daqifi.Desktop.UpdateVersion.VersionNotification 0% 0%
Daqifi.Desktop.View.AddChannelDialog 0% 0%
Daqifi.Desktop.View.AddProfileConfirmationDialog 0% 0%
Daqifi.Desktop.View.AddprofileDialog 0% 0%
Daqifi.Desktop.View.ConnectionDialog 0% 0%
Daqifi.Desktop.View.DebugWindow 0% 0%
Daqifi.Desktop.View.DeviceLogsView 0% 0%
Daqifi.Desktop.View.DuplicateDeviceDialog 0% 0%
Daqifi.Desktop.View.ErrorDialog 0% 0%
Daqifi.Desktop.View.ExportDialog 0% 0%
Daqifi.Desktop.View.FirmwareDialog 0% 0%
Daqifi.Desktop.View.Flyouts.ChannelsFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.DevicesFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.FirmwareFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.LiveGraphFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.LoggedSessionFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.NotificationsFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.SummaryFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.UpdateProfileFlyout 0% 0%
Daqifi.Desktop.View.MinimapInteractionController 0% 0%
Daqifi.Desktop.View.SelectColorDialog 0% 0%
Daqifi.Desktop.View.SettingsDialog 0% 0%
Daqifi.Desktop.View.SuccessDialog 0% 0%
Daqifi.Desktop.ViewModels.AddChannelDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.AddProfileConfirmationDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.AddProfileDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.ConnectionDialogViewModel 21.7% 19.5%
Daqifi.Desktop.ViewModels.DaqifiViewModel 14.6% 8.1%
Daqifi.Desktop.ViewModels.DeviceLogsViewModel 0% 0%
Daqifi.Desktop.ViewModels.DeviceSettingsViewModel 0% 0%
Daqifi.Desktop.ViewModels.DuplicateDeviceDialogViewModel 0%
Daqifi.Desktop.ViewModels.ErrorDialogViewModel 0%
Daqifi.Desktop.ViewModels.ExportDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.FirmwareDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.SelectColorDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.SettingsViewModel 0%
Daqifi.Desktop.ViewModels.SuccessDialogViewModel 85.7%
Daqifi.Desktop.WindowViewModelMapping.IWindowViewModelMappingsContract 0%
Daqifi.Desktop.WindowViewModelMapping.WindowViewModelMappings 0%
Sentry.Generated.BuildPropertyInitializer 100%
Daqifi.Desktop.Common - 30.8%
Name Line Branch
Daqifi.Desktop.Common 30.8% 16.6%
Daqifi.Desktop.Common.Loggers.AppLogger 33.7% 16.6%
Daqifi.Desktop.Common.Loggers.NoOpLogger 0%
Daqifi.Desktop.IO - 100%
Name Line Branch
Daqifi.Desktop.IO 100% ****
Daqifi.Desktop.IO.Messages.MessageEventArgs`1 100%

Coverage report generated by ReportGeneratorView full report in build artifacts

@tylerkron tylerkron merged commit a7deff5 into main Apr 11, 2026
2 checks passed
@tylerkron tylerkron deleted the claude/hungry-mayer branch April 11, 2026 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Overview Minimap for Large Dataset Navigation in Plot View

1 participant