Sort unrecorded years last, and stop claiming a copy that failed

Both reported by Codex on #22, both confirmed before fixing.

normaliseWine collapses a missing year to 0, and the missing-value rule
only recognised null and "". So 0 sorted as a number smaller than every
real year, and sorting by vintage ascending led with every NV bottle in
the cellar - the exact thing that rule exists to prevent. Zero now counts
as absent for Vintage, BeginConsume and EndConsume, where there is no
such year. Valuation is deliberately left alone: a bottle really can be
worth nothing recorded, and cheapest-first should still show it.

The copy button reported success unconditionally. `navigator.clipboard`
requires a secure context, and a great many Home Assistant installs are
reached over plain http on a LAN address - so `written` was undefined,
the else branch ran, and the button said "Copied" having copied nothing.
That is worse than doing nothing: the user walks to the rack and pastes
whatever was there before. There is a real execCommand fallback now, and
the label follows what actually happened.

Finding the second one meant fixing the harness first. It set
`globalThis.navigator`, which node 21+ ignores in silence because it
ships its own getter-only navigator - so every clipboard fixture was
testing the same world and the "missing" case looked like it passed. It
uses defineProperty now and throws if the stub does not take.

The copy tests assert what reached the clipboard, not just what the
button says, so "said Copied" cannot pass for "copied something".

441 tests, ruff clean, mypy clean. Re-checked in Chromium: vintage
ascending now ends with the NV bottles, and Copy bin really does put the
bin on the clipboard.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLPEGFSy3fLEuXNUPAPWR4

Author: claude

custom_components/cellar_tracker/www/cellar.html

@@ -461,6 +461,19 @@
         return value == null || value === '';
     }
 
+    // normaliseWine collapses an unrecorded year to 0, and 0 is smaller than
+    // every real one - so sorting by vintage ascending led with every NV
+    // bottle in the cellar. There is no year zero, so 0 means "not recorded"
+    // for these three and nothing else. A *valuation* of 0 is left alone: a
+    // bottle really can be worth nothing recorded, and cheapest-first should
+    // still show it.
+    const UNRECORDED_AS_ZERO = { Vintage: true, BeginConsume: true, EndConsume: true };
+
+    function isMissing(value, key) {
+        if (isBlank(value)) return true;
+        return UNRECORDED_AS_ZERO[key] === true && Number(value) === 0;
+    }
+
     // Decorated with the arrival index and compared on it last, so ties keep
     // the order they came in whichever way the column is pointing. A cellar
     // holds six identical bottles of the same wine; they should not shuffle
@@ -475,9 +488,11 @@
 
                 // A bin nobody filled in sorts last in both directions rather
                 // than heading the list because "" precedes every letter.
-                if (isBlank(left) || isBlank(right)) {
-                    if (isBlank(left) && isBlank(right)) return a.index - b.index;
-                    return isBlank(left) ? 1 : -1;
+                const leftMissing = isMissing(left, key);
+                const rightMissing = isMissing(right, key);
+                if (leftMissing || rightMissing) {
+                    if (leftMissing && rightMissing) return a.index - b.index;
+                    return leftMissing ? 1 : -1;
                 }
 
                 if (typeof left === 'string') left = left.toLowerCase();
@@ -695,16 +710,45 @@
         return drawer;
     }
 
-    function copyText(text, button, original) {
-        const done = () => {
-            button.textContent = 'Copied';
-            setTimeout(() => { button.textContent = original; }, 1200);
-        };
+    // execCommand is deprecated, and it is also the only thing that works
+    // without a secure context. A great many Home Assistant installs are
+    // reached over plain http on a LAN address, where navigator.clipboard does
+    // not exist at all - so this is the common path here, not the fallback.
+    function legacyCopy(text) {
         try {
-            const written = navigator.clipboard && navigator.clipboard.writeText(text);
-            if (written && written.then) written.then(done, () => {});
-            else done();
-        } catch (e) { /* no clipboard permission; the bin is on screen anyway */ }
+            const field = document.createElement('textarea');
+            field.value = text;
+            field.setAttribute('readonly', '');
+            field.style.position = 'fixed';
+            field.style.opacity = '0';
+            document.body.appendChild(field);
+            field.select();
+            const copied = document.execCommand('copy') === true;
+            document.body.removeChild(field);
+            return copied;
+        } catch (e) {
+            return false;
+        }
+    }
+
+    // Resolves to whether the text actually reached a clipboard. Saying
+    // "Copied" when nothing was copied is worse than saying nothing: the user
+    // walks to the rack and pastes whatever was there before.
+    function writeToClipboard(text) {
+        if (navigator.clipboard && navigator.clipboard.writeText) {
+            try {
+                return Promise.resolve(navigator.clipboard.writeText(text))
+                    .then(() => true, () => legacyCopy(text));
+            } catch (e) { /* fall through to the legacy path */ }
+        }
+        return Promise.resolve(legacyCopy(text));
+    }
+
+    function copyText(text, button, original) {
+        writeToClipboard(text).then((copied) => {
+            button.textContent = copied ? 'Copied' : 'Could not copy';
+            setTimeout(() => { button.textContent = original; }, copied ? 1200 : 2000);
+        });
     }
 
     function buildBottle(wine) {

tests/dashboard_js.py

@@ -84,6 +84,7 @@ def page_script() -> str:
         querySelectorAll() { return []; },
         closest() { return null; },
         focus() {},
+        select() { el.selected = true; },
     };
     return el;
 }
@@ -114,9 +115,44 @@ def page_script() -> str:
     createDocumentFragment: () => makeElement('#fragment'),
     addEventListener() {},
     body: makeElement('body'),
+    // The legacy path, and the only one that works without a secure context.
+    execCommand(command) {
+        if (fixture.execCommand === false) return false;
+        if (command === 'copy') copied.push('exec');
+        return fixture.execCommand !== false;
+    },
 };
 
-globalThis.navigator = { clipboard: { writeText: async () => {} } };
+// Home Assistant is very often served over plain http on a LAN address, which
+// is not a secure context - so navigator.clipboard is frequently absent
+// entirely. The fixture picks which world the page is running in.
+const copied = [];
+const clipboard = fixture.clipboard || 'async';
+// defineProperty, not assignment: node 21+ ships its own `navigator` as a
+// getter-only global, so `globalThis.navigator = ...` silently does nothing
+// and every clipboard fixture would quietly test the same world.
+Object.defineProperty(globalThis, 'navigator', {
+    configurable: true,
+    writable: true,
+    value: clipboard === 'missing'
+        ? {}
+        : {
+            clipboard: {
+                writeText: (text) => clipboard === 'rejects'
+                    ? Promise.reject(new Error('denied'))
+                    : (copied.push(text), Promise.resolve()),
+            },
+        },
+});
+
+// The stubs above are worthless if the runtime refused one of them, and a
+// refusal is silent outside strict mode. Fail loudly instead.
+if (clipboard === 'missing' && navigator.clipboard) {
+    throw new Error('the navigator stub did not take effect');
+}
+if (clipboard !== 'missing' && !navigator.clipboard) {
+    throw new Error('the navigator stub did not take effect');
+}
 
 // The page fetches at load. Answer with the fixture so the render path runs;
 // an empty fixture answers 401, which is the existing tests' scenario.
@@ -138,9 +174,13 @@ def page_script() -> str:
 """
 
 
-def run_js(checks: str, *, wines=None, search="") -> str:
+def run_js(checks: str, *, wines=None, search="", clipboard="async",
+           exec_command=True) -> str:
     """Load the page script, then run `checks`. Non-zero exit means a failure."""
-    fixture = json.dumps({"wines": wines, "search": search})
+    fixture = json.dumps(
+        {"wines": wines, "search": search,
+         "clipboard": clipboard, "execCommand": exec_command}
+    )
     source = "\n".join(
         [
             PRELUDE % fixture,

tests/test_dashboard_filters.py

@@ -328,3 +328,43 @@ def test_rows_missing_the_sort_key_sort_last_in_both_directions():
         + equals("sortWines(rows, 'Bin', 'desc').map((w) => w.Wine)",
                  ["A", "C", "B"], "a missing bin should sort last descending too")
     )
+
+
+# --------------------------------------------------------------------------
+# Reported by Codex on #22
+# --------------------------------------------------------------------------
+UNRECORDED = (
+    "[{Wine: 'Real Vintage', Vintage: 2019, EndConsume: 2030},"
+    " {Wine: 'No Vintage', Vintage: 0, EndConsume: 0},"
+    " {Wine: 'Older', Vintage: 2010, EndConsume: 2020}]"
+)
+
+
+@pytest.mark.parametrize("key", ["Vintage", "EndConsume"])
+@pytest.mark.parametrize("direction", ["asc", "desc"])
+def test_an_unrecorded_year_sorts_last_whichever_way_the_column_points(key, direction):
+    """normaliseWine collapses a missing year to 0, and 0 is smaller than 2010.
+
+    So an ascending sort by vintage led with every NV bottle in the cellar -
+    the exact thing the missing-value rule was written to prevent, slipping
+    through because the rule only recognised null and "".
+    """
+    run_js(
+        equals(
+            f"sortWines({UNRECORDED}, '{key}', '{direction}').slice(-1)[0].Wine",
+            "No Vintage",
+            f"a bottle with no recorded {key} should sort last, not first",
+        )
+    )
+
+
+def test_a_valuation_of_zero_is_still_a_value():
+    """Unlike a year: there is no year 0, but a bottle really can be worth 0."""
+    run_js(
+        equals(
+            "sortWines([{Wine: 'Free', Valuation: 0}, {Wine: 'Costly', Valuation: 99}],"
+            " 'Valuation', 'asc').map((w) => w.Wine)",
+            ["Free", "Costly"],
+            "a zero valuation was treated as missing rather than as cheap",
+        )
+    )

tests/test_dashboard_render.py

@@ -216,3 +216,61 @@ def test_an_empty_result_says_so_instead_of_showing_nothing():
         + check("document.getElementById('status').textContent.length > 0",
                 "an empty list with no explanation reads as a broken page")
     )
+
+
+# --------------------------------------------------------------------------
+# Reported by Codex on #22
+# --------------------------------------------------------------------------
+def copy_scenario(clipboard: str, exec_command: bool) -> dict:
+    """Open a drawer, press Copy bin, and report the label and what was copied."""
+    output = run_js(
+        f"{PREAMBLE}\n{FIND}\n"
+        f"loadWines({json.dumps(cellar())});\n"
+        f"const bottle = {card('Ready Red')};\n"
+        "bottle.children[0].dispatch('click');\n"
+        "const copy = bottle.children[1].children[1].children[0];\n"
+        "copy.dispatch('click', {stopPropagation() {}});\n"
+        # Report the label and what actually reached a clipboard, so a test can
+        # tell "said Copied" apart from "copied something".
+        "setTimeout(() => {\n"
+        "  console.log(JSON.stringify({label: copy.textContent, copied: copied}));\n"
+        "}, 10);\n",
+        wines=None,
+        clipboard=clipboard,
+        exec_command=exec_command,
+    )
+    return json.loads(output.strip())
+
+
+def test_copying_says_so_when_it_worked():
+    result = copy_scenario("async", True)
+    assert result["label"] == "Copied"
+    assert "A1" in result["copied"], "the bin never reached the clipboard"
+
+
+def test_a_missing_clipboard_api_falls_back_rather_than_failing():
+    """Home Assistant on plain http is not a secure context, so there is no
+    navigator.clipboard at all - the common case here, not an edge one."""
+    result = copy_scenario("missing", True)
+    assert result["label"] == "Copied"
+    assert result["copied"], "claimed success without using the fallback"
+
+
+def test_a_rejected_clipboard_write_still_falls_back():
+    result = copy_scenario("rejects", True)
+    assert result["label"] == "Copied"
+    assert result["copied"], "gave up instead of trying the legacy path"
+
+
+def test_the_button_does_not_claim_success_when_nothing_was_copied():
+    """It used to say "Copied" whenever the clipboard API was absent."""
+    result = copy_scenario("missing", False)
+    assert not result["copied"], "the fixture should model a copy that fails"
+    assert result["label"] != "Copied"
+
+
+def test_a_failed_copy_says_what_happened():
+    label = copy_scenario("missing", False)["label"]
+    assert label and label != "Copy bin", (
+        "a copy that silently did nothing leaves the user believing it worked"
+    )