Severity: Info

Summary

MonitoringViewModelBase declares StopMonitoring as a public virtual method with no parameters. Two derived classes add an overload with a bool parameter but do not use the override keyword, and do not override the base signature either — the derived declarations silently hide the base method. This compiles but triggers a C# warning (CS0108) and creates a footgun: any caller holding a MonitoringViewModelBase reference will never get the clear/reset behavior the derived classes expect.

File and lines

Base — src/Servy.Manager/ViewModels/MonitoringViewModelBase.cs:135:

public virtual void StopMonitoring()
{
    _monitoringCts?.Cancel();
    Interlocked.Exchange(ref _isMonitoringFlag, 0);
    _timer?.Stop();
}

Derived — src/Servy.Manager/ViewModels/ConsoleViewModel.cs:609:

public void StopMonitoring(bool clearConsole)
{
    base.StopMonitoring();

    if (clearConsole)
    {
        ResetConsole(true);
    }
}

Derived — src/Servy.Manager/ViewModels/PerformanceViewModel.cs:402:

public void StopMonitoring(bool clearPoints)
{
    base.StopMonitoring();

    if (clearPoints)
    {
        // Clear points
    }
}

Neither derived method uses the override keyword. Because the signatures differ (extra bool parameter), this is a new method that shadows the inherited one, not an override.

Explanation

The intent of the derived signature is to extend StopMonitoring() with an optional "clear UI state" behavior. But because it is a new method — not an override — polymorphic callers see only the base version:

MonitoringViewModelBase vm = console; // base-typed reference
vm.StopMonitoring();                  // calls base only; clearConsole path never runs

In the current codebase all call sites use the derived-class type (ConsoleViewModel / PerformanceViewModel) directly, so there is no functional bug today. The concern is:

1. The intent is unclear to readers — a virtual base method plus a similarly-named (but non-override) derived method looks like a bug waiting to happen.
2. The new keyword is missing, so the compiler emits warning CS0108 in projects that promote warnings to errors.
3. Any future refactor that starts to treat instances polymorphically (interface introduction, command routing, test double) will silently lose the reset behavior.

Suggested fix

Pick one of:

Option 1 — override without the parameter, add a separate reset entry point:

// base
public virtual void StopMonitoring() { ... }

// derived
public override void StopMonitoring() { base.StopMonitoring(); }
public void StopMonitoringAndClear() { StopMonitoring(); ResetConsole(true); }

Option 2 — make the base virtual method take the flag:

// base
public virtual void StopMonitoring(bool clearView = false)
{
    _monitoringCts?.Cancel();
    Interlocked.Exchange(ref _isMonitoringFlag, 0);
    _timer?.Stop();
}

// derived
public override void StopMonitoring(bool clearView = false)
{
    base.StopMonitoring(clearView);
    if (clearView) ResetConsole(true);
}

Option 2 is cleaner because it matches the way both derived classes are already calling it (StopMonitoring(false)) at ConsoleViewModel:162 and PerformanceViewModel:74.