Address review feedback: clamp threshold, normalize hex, fix isLightBackground, validate gradient palette, fix test names

Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
Agent-Logs-Url: https://github.com/jongalloway/DiagramForge/sessions/4b46141f-444c-46cc-9873-d40f91adb1a3

Author: Copilot

src/DiagramForge/Models/ColorUtils.cs

@@ -170,10 +170,11 @@ public static double GetContrastRatio(string hex1, string hex2)
     /// <param name="hex">Hex color string.</param>
     /// <param name="saturationThreshold">
     /// Saturation threshold 0–1; colors with saturation below this value are considered
-    /// achromatic. Defaults to 0.08.
+    /// achromatic. Values outside [0,1] are clamped. Defaults to 0.08.
     /// </param>
     public static bool IsAchromatic(string hex, double saturationThreshold = 0.08)
     {
+        saturationThreshold = Math.Clamp(saturationThreshold, 0, 1);
         var (rRaw, gRaw, bRaw) = ParseHex(hex);
         double r = rRaw / 255d;
         double g = gRaw / 255d;
@@ -215,13 +216,20 @@ public static bool IsPaletteMonochrome(IReadOnlyList<string> palette, string? ba
             return true;
 
         // All entries are the same color (trivially monochrome — no hue variety).
-        if (palette.All(c => string.Equals(c, palette[0], StringComparison.OrdinalIgnoreCase)))
-            return true;
+        // Compare by parsed RGBA values so shorthand formats (#FFF vs #FFFFFF) are treated as equal.
+        {
+            var referenceColor = ParseHexWithAlpha(palette[0]);
+            if (palette.All(c => ParseHexWithAlpha(c) == referenceColor))
+                return true;
+        }
 
         // All entries match the background (nodes would be invisible against the canvas).
-        if (backgroundColor is not null &&
-            palette.All(c => string.Equals(c, backgroundColor, StringComparison.OrdinalIgnoreCase)))
-            return true;
+        if (backgroundColor is not null)
+        {
+            var background = ParseHexWithAlpha(backgroundColor);
+            if (palette.All(c => ParseHexWithAlpha(c) == background))
+                return true;
+        }
 
         return false;
     }

src/DiagramForge/Models/ThemePaletteResolver.cs

@@ -27,7 +27,7 @@ public static class ThemePaletteResolver
     ///   <item>
     ///     If <see cref="Theme.UseBorderGradients"/> is <see langword="true"/> and
     ///     <see cref="Theme.BorderGradientStops"/> contains more than one stop, the stops
-    ///     are interpolated/cycled to produce <see cref="DefaultPaletteSize"/> entries.
+    ///     are linearly interpolated to produce <see cref="DefaultPaletteSize"/> entries.
     ///   </item>
     ///   <item>
     ///     Otherwise, a palette is derived from <see cref="Theme.AccentColor"/> and
@@ -52,10 +52,15 @@ public static IReadOnlyList<string> ResolveEffectivePalette(Theme theme)
 
         // Build fallback from gradient stops when they are available and meaningful.
         if (theme.UseBorderGradients && theme.BorderGradientStops is { Count: > 1 })
-            return BuildPaletteFromGradientStops(theme.BorderGradientStops, DefaultPaletteSize);
+        {
+            var gradientPalette = BuildPaletteFromGradientStops(theme.BorderGradientStops, DefaultPaletteSize);
+            if (!ColorUtils.IsPaletteMonochrome(gradientPalette, theme.BackgroundColor))
+                return gradientPalette;
+        }
 
         // Fall back to hue-rotation derivation from the theme's semantic colors.
-        return BuildPaletteFromHueRotation(theme.AccentColor, theme.SecondaryColor, DefaultPaletteSize);
+        bool isLightBackground = ColorUtils.IsLight(theme.BackgroundColor);
+        return BuildPaletteFromHueRotation(theme.AccentColor, theme.SecondaryColor, DefaultPaletteSize, isLightBackground);
     }
 
     // ── Private helpers ───────────────────────────────────────────────────────
@@ -97,17 +102,16 @@ private static IReadOnlyList<string> BuildPaletteFromGradientStops(IReadOnlyList
     /// in equal steps, alternating between the two base colors.
     /// </summary>
     private static IReadOnlyList<string> BuildPaletteFromHueRotation(
-        string accentColor, string secondaryColor, int count)
+        string accentColor, string secondaryColor, int count, bool isLightBackground)
     {
-        bool isLight = ColorUtils.IsLight(accentColor);
         double hueStep = 360.0 / count;
         var result = new List<string>(count);
 
         for (int i = 0; i < count; i++)
         {
             string baseColor = i % 2 == 0 ? accentColor : secondaryColor;
             double rotation = Math.Floor(i / 2.0) * hueStep;
-            result.Add(ColorUtils.RotateHue(baseColor, rotation, isLight));
+            result.Add(ColorUtils.RotateHue(baseColor, rotation, isLightBackground));
         }
 
         return result;

tests/DiagramForge.Tests/Models/ColorUtilsTests.cs

@@ -724,7 +724,7 @@ public void IsAchromatic_LowSaturationPastel_DefaultThreshold_ReturnsTrue()
     }
 
     [Fact]
-    public void IsAchromatic_CustomThreshold_LowSaturationColor_ReturnsTrue()
+    public void IsAchromatic_CustomThreshold_LowSaturationColor_ReturnsFalse()
     {
         // A muted blue — below the default threshold but above an even lower custom one
         // Test that the threshold parameter is respected
@@ -804,14 +804,30 @@ public void IsPaletteMonochrome_MatchesBackground_ReturnsTrue()
     }
 
     [Fact]
-    public void IsPaletteMonochrome_MatchesBackground_NoBackground_ReturnsFalse()
+    public void IsPaletteMonochrome_MatchesBackground_NoBackground_ReturnsTrue()
     {
         // Same chromatic entries, but without a background to compare against —
         // they are the same color, so still "all same color" → true
         var palette = new[] { "#2563EB", "#2563EB", "#2563EB" };
         Assert.True(ColorUtils.IsPaletteMonochrome(palette, backgroundColor: null));
     }
 
+    [Fact]
+    public void IsPaletteMonochrome_ShorthandVsFullEquivalent_ReturnsTrue()
+    {
+        // #FFF and #FFFFFF are the same color — palette should be detected as monochrome
+        var palette = new[] { "#FFF", "#FFFFFF", "#FFF" };
+        Assert.True(ColorUtils.IsPaletteMonochrome(palette));
+    }
+
+    [Fact]
+    public void IsPaletteMonochrome_ShorthandBackground_NormalizedComparison_ReturnsTrue()
+    {
+        // #FFF (shorthand) vs #FFFFFF (full) should match background correctly
+        var palette = new[] { "#FFFFFF", "#FFFFFF" };
+        Assert.True(ColorUtils.IsPaletteMonochrome(palette, backgroundColor: "#FFF"));
+    }
+
     [Fact]
     public void IsPaletteMonochrome_PrismTheme_ReturnsTrue()
     {
@@ -821,7 +837,24 @@ public void IsPaletteMonochrome_PrismTheme_ReturnsTrue()
         Assert.True(ColorUtils.IsPaletteMonochrome(prism.NodePalette, prism.BackgroundColor));
     }
 
-    // ── Vibrant (achromatic hardening) ────────────────────────────────────────
+    // ── IsAchromatic threshold clamping ───────────────────────────────────────
+
+    [Fact]
+    public void IsAchromatic_NegativeThreshold_ClampsToZero_ReturnsFalse()
+    {
+        // Threshold clamped to 0 — no color can be below a 0 saturation threshold
+        Assert.False(ColorUtils.IsAchromatic("#808080", saturationThreshold: -1.0));
+    }
+
+    [Fact]
+    public void IsAchromatic_ThresholdAbove1_ClampsTo1_ReturnsTrue()
+    {
+        // Threshold clamped to 1 — any color with saturation < 1 is considered achromatic.
+        // #CC5050 is a muted red with HSL saturation ~0.5, so it returns true when
+        // the (clamped) threshold is 1.0, even though it is chromatic at the default threshold.
+        Assert.True(ColorUtils.IsAchromatic("#CC5050", saturationThreshold: 2.0));
+        Assert.False(ColorUtils.IsAchromatic("#CC5050")); // default threshold does not treat it as achromatic
+    }
 
     [Fact]
     public void Vibrant_WhiteInput_ProducesVisiblyDifferentColor()

tests/DiagramForge.Tests/Models/ThemePaletteResolverTests.cs

@@ -275,4 +275,21 @@ public void ResolveEffectivePalette_MixedMonochromeGrays_ReturnsFallback()
         Assert.False(ColorUtils.IsPaletteMonochrome(result),
             "Fallback for mixed-gray palette should not be monochrome.");
     }
+
+    [Fact]
+    public void ResolveEffectivePalette_MonochromeGradientStops_FallsBackToHueRotation()
+    {
+        // Even when UseBorderGradients is true, if the gradient stops are themselves
+        // monochrome the method should skip them and fall back to hue rotation.
+        var theme = Theme.FromPalette("#3B82F6");
+        theme.NodePalette = ["#FFFFFF", "#FFFFFF"];
+        theme.UseBorderGradients = true;
+        theme.BorderGradientStops = ["#808080", "#AAAAAA"];  // achromatic stops
+
+        var result = ThemePaletteResolver.ResolveEffectivePalette(theme);
+
+        Assert.Equal(8, result.Count);
+        Assert.False(ColorUtils.IsPaletteMonochrome(result),
+            "Result should be chromatic even when gradient stops are monochrome.");
+    }
 }