fix(listview): ids must also be unique and not NaN

The scalar contract said "identity" and enforced only "scalar", so two
ways to hold an id that is not one survived.

NaN passes `type(x) == "number"` and then errors at
`p.collapsed[row.id]` with "table index is NaN" — the one scalar Lua
accepts as a number and refuses as a key. Bitten with the check
removed, it reports exactly that, from inside listview, naming no row.

DUPLICATES do not merely collide. Every lookup here — `line_of_id`, and
toggle's scan for the row index — resolves an id to the FIRST row
bearing it, so selecting the second such row toggles the first and
re-seats the cursor onto it: a stray jump with nothing pointing at the
id. Bitten with the check removed, nothing is raised at all.

Both are enforced in `check_ids`, where rows already enter, so the
error names the offending row (and, for a duplicate, both of them)
instead of surfacing as a low-level error or a wrong jump later. The
error text says why, not just what, since the reason is not guessable
from the rule.

Verified: fmt, clippy, diff-check, --lib 1897/0, crdt 2082/0, listview
26/26, m4 150/0, gpu 221/0, bottom_panel_stage1 47/47.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

builtin/runtime/listview.lua

@@ -158,26 +158,58 @@ local function hidden_by_ancestor(p, rows, i)
   return false
 end
 
--- Ids must be scalars, and the reason is not fussiness about types.
--- Selection compares them with `==`, which honours `__eq`; collapse
--- state stores them as TABLE KEYS, and Lua indexes tables by raw
--- identity, consulting no metamethod. A table id would therefore
+-- Ids must be usable, unique table keys, and none of the three checks
+-- below is fussiness about types.
+--
+-- SCALAR. Selection compares ids with `==`, which honours `__eq`;
+-- collapse state stores them as TABLE KEYS, and Lua indexes tables by
+-- raw identity, consulting no metamethod. A table id would therefore
 -- satisfy one and quietly fail the other: after a refresh minted fresh
 -- id tables, selection would be restored and the fold would be lost.
 --
 -- Equality-aware collapse lookup is the alternative, and it is worse
 -- here: `hidden_by_ancestor` runs per row and would turn a linear
--- render quadratic to support a key type no consumer has wanted. So
--- the contract is narrowed to the one both halves can honour, and
--- enforced where rows enter rather than discovered as a lost fold.
+-- render quadratic to support a key type no consumer has wanted.
+--
+-- NOT NaN. `0/0` passes a `type(x) == "number"` test and then *errors*
+-- at `p.collapsed[row.id]` with "table index is NaN" — the one scalar
+-- Lua accepts as a number and refuses as a key. Caught here so the
+-- report names the row, rather than surfacing on whichever later TAB
+-- happens to reach it.
+--
+-- UNIQUE. Every lookup here resolves an id to the FIRST row bearing
+-- it, so duplicates do not merely collide: selecting the second such
+-- row toggles the first and re-seats the cursor onto it. An id that
+-- does not identify a node is not an id, and the contract's word for
+-- itself is identity.
+--
+-- All three are enforced where rows enter, so a bad id is a named
+-- error at the call site instead of a lost fold or a stray jump later.
 local function check_ids(rows)
+  local seen = {}
   for i, row in ipairs(rows) do
-    local k = type(row.id)
-    if row.id ~= nil and k ~= "string" and k ~= "number" then
-      error(string.format(
-        "listview: row %d has a %s id; ids must be a string or number "
-        .. "(collapse state keys a table by identity, so a %s id would "
-        .. "lose its fold across a refresh)", i, k, k))
+    local id, k = row.id, type(row.id)
+    if id ~= nil then
+      if k ~= "string" and k ~= "number" then
+        error(string.format(
+          "listview: row %d has a %s id; ids must be a string or number "
+          .. "(collapse state keys a table by identity, so a %s id would "
+          .. "lose its fold across a refresh)", i, k, k))
+      end
+      if id ~= id then
+        error(string.format(
+          "listview: row %d has a NaN id; NaN is a number but not a "
+          .. "usable table key, and collapse state would raise "
+          .. "\"table index is NaN\" on the first fold", i))
+      end
+      if seen[id] then
+        error(string.format(
+          "listview: rows %d and %d share the id %q; ids must be unique "
+          .. "(every lookup resolves to the first match, so selecting "
+          .. "the later row would toggle and re-seat the earlier one)",
+          seen[id], i, tostring(id)))
+      end
+      seen[id] = i
     end
   end
   return rows

docs/tree-primitive-framing.md

@@ -21,7 +21,7 @@ toward the consumer keeping `text`.
 |---|---|
 | **Q#TR1** | **Extend `listview`.** A separate `treeview` would either duplicate ~200 lines of panel discipline (Q#GB18 handle identity, Q#GB13 `<2>` disambiguation, the read-only intercept, `prev` capture, quit chain, generated-buffer writes) or require *extracting* them from a shipped primitive first — the riskier change. Extending is backward-compatible by construction: absent `depth`/`id` give today's behaviour exactly, which the three flat consumers already produce. |
 | **Q#TR2** | **Primitive-owned collapse state**, keyed by row id, held in the panel record. Consumer-owned would make every consumer reimplement refresh survival. |
-| **Q#TR3** | **Consumer-supplied `row.id`**, a **string or number**, compared by value; the primitive never derives one. *(Review narrowed this from "opaque": collapse state keys a table, and Lua table indexing ignores `__eq`, so an opaque id could restore selection while silently losing its fold. `check_ids` enforces it where rows enter.)* The outline uses **`line:col`** — unique per document and stable across re-render, where the `::` parent chain collides on overloads. |
+| **Q#TR3** | **Consumer-supplied `row.id`**, a **string or number**, compared by value; the primitive never derives one. *(Review narrowed this from "opaque": collapse state keys a table, and Lua table indexing ignores `__eq`, so an opaque id could restore selection while silently losing its fold. `check_ids` enforces it where rows enter — and with it that ids are **unique** and **not NaN**, since every lookup takes the first match (so a duplicate makes selecting the later row toggle the earlier) and `0/0` is the one scalar Lua counts as a number and refuses as a table key.)* The outline uses **`line:col`** — unique per document and stable across re-render, where the `::` parent chain collides on overloads. |
 | **Q#TR4** | **Consumer keeps pre-rendered `text`**; `depth` is structural only. Also sidesteps the conflict with dired's fixed-width `_layout` column contract when it adopts. |
 
 **Acceptance 5 is decided too: byte-identity coverage is written**,

tests/listview_acceptance.rs

@@ -1144,6 +1144,58 @@ fn tr_6_a_non_scalar_id_is_rejected_where_rows_enter() {
     );
 }
 
+/// A NaN id passes `type(x) == "number"` and then errors at
+/// `p.collapsed[row.id]` with "table index is NaN" — the one scalar
+/// Lua accepts as a number and refuses as a table key. It must be
+/// caught where rows enter, naming the row, rather than surfacing on
+/// whichever later TAB happens to reach it.
+#[test]
+fn tr_7_a_nan_id_is_rejected_rather_than_erroring_on_the_first_fold() {
+    let s = editor();
+    let err: String = eval(
+        &s,
+        r#"local ok, e = pcall(function()
+             pmacs.listview.open {
+               name = "*tr7*",
+               header = "h",
+               rows = { { text = "a", depth = 0, id = 0 / 0 } },
+             }
+           end)
+           return tostring(e)"#,
+    );
+    assert!(
+        err.contains("NaN id"),
+        "named at entry, not as a table-index error later: {err}"
+    );
+}
+
+/// Duplicate ids do not merely collide — every lookup resolves an id to
+/// the FIRST row bearing it, so selecting the second toggles the first
+/// and re-seats the cursor onto it. An id that does not identify a node
+/// is not an id.
+#[test]
+fn tr_8_duplicate_ids_are_rejected_because_lookup_takes_the_first_match() {
+    let s = editor();
+    let err: String = eval(
+        &s,
+        r#"local ok, e = pcall(function()
+             pmacs.listview.open {
+               name = "*tr8*",
+               header = "h",
+               rows = {
+                 { text = "first",  depth = 0, id = "same" },
+                 { text = "second", depth = 0, id = "same" },
+               },
+             }
+           end)
+           return tostring(e)"#,
+    );
+    assert!(
+        err.contains("share the id") && err.contains("rows 1 and 2"),
+        "both offending rows named: {err}"
+    );
+}
+
 // Isolated bootstrap storage roots (see the module docs): an
 // integration test is compiled without `cfg(test)`, so a raw
 // `EditorState::new()` would read the developer's real `init.lua` and