test(e2e): un-skip add_row + bulk_remove flakes — root cause was test bugs

Previous "state-machine anomaly" skips (logged in qa-findings.jsonl
2026-05-24) were actually three stacked TEST bugs, not app bugs:

1. **Pagination trap (test_wl_save.py + test_crud_workflow.py)**:
   `wl_table.js:776` jumps `currentPage` to the new last page after
   add_row. The new row is the LAST visible row on that page (not the
   first). Tests filling `set_cell_value(0, ...)` instead overwrote
   existing rows on the new page, and the empty new row got filtered
   out by `wl_save.js:505` (drops rows where all visible cols are
   empty). Net effect: zero row growth + one silent edit of an
   unrelated row.

2. **Comment-column gate (test_crud_workflow.py)**: `getAuditComment`
   at `wl_save.js:410` does NOT show an audit-comment modal when the
   CSV has a `Comment` column. Instead it scans EVERY visible row's
   Comment cell — if any cell is empty (including the new row), the
   entire save aborts client-side with "Comment field cannot be empty".
   Tests must fill the Comment cell on the new row.

3. **Read-only Expires cell (test_wl_save.py)**: the `Expires` column
   uses a date-picker overlay; the underlying input is read-only.
   `textarea.fill()` fails with "element is not editable". Tests must
   skip `Expires` (and any `_`-prefixed reserved cols).

4. **Paginated DOM count assertions (both files)**: assertions like
   `final < initial` or `new_count == initial + 1` on paginated tables
   are unreliable — `reloadCsvQuiet` after save returns to page 0,
   which still shows ROWS_PER_PAGE rows even when the total decreased
   by 2. Replaced with REST `get_csv_content` verification of the
   true row count.

Fixed tests (all four were skip-marked, now active):
- tests/e2e/test_crud_workflow.py::test_add_row
- tests/e2e/test_crud_workflow.py::test_crud_workflow_end_to_end
- tests/e2e/test_wl_save.py::TestAddRowSave::test_add_row_and_save
- tests/e2e/test_wl_save.py::TestBulkRemoval::test_bulk_remove

Full Python E2E baseline after this commit (playwright 1.60 + chromium 148):
  30 passed, 9 skipped, 0 errors
  (was: 26 passed, 11 skipped, 2 failed)

Notes:
- The DR55_brute_force_users.csv had 2 polluted rows from prior REST
  probe sessions (empty-Comment rows that triggered the
  getAuditComment gate). Cleanup was a one-time REST call; the
  fixed tests use `backup_csv`/`restore_csv` and won't reintroduce
  pollution.
- The 9 remaining skips are unchanged: multi-tab CP iteration flake
  (2), multi-user approval flow deferred to .cjs (4), revert UI
  flow (3) — the last 3 can be implemented now that save-persistence
  works, but that's separate scope.

Author: RelativisticJet

tests/e2e/test_crud_workflow.py

@@ -81,24 +81,30 @@ def test_edit_cell_and_save(browser, rest_client):
 
 @pytest.mark.crud
 @pytest.mark.e2e
-@pytest.mark.skip(reason=(
-    "Known pre-existing state-machine issue: after add_row + set_cell_value(new_value) "
-    "+ save_changes, REST shows row count unchanged. Same failure mode exists in "
-    "test_wl_save.py::test_add_row_and_save (10 -> 2 / 10 -> 1 outcomes). Suspected "
-    "interaction with content_hash optimistic locking when the only change is a new "
-    "row with mostly-empty fields; needs dedicated investigation against the save "
-    "pipeline. Not a playwright 1.60 regression (also fails on 1.40 per the same "
-    "smoke). See qa-findings.jsonl 2026-05-24 entry."
-))
 def test_add_row(browser, rest_client):
-    """Add a row, fill the first column, save. Verify row added via REST.
-
-    NOTE on pagination: `add_row` jumps the table to the LAST page (where
-    the new empty row lives — see wl_table.js:776
-    `currentPage = ceil(currentRows.length / ROWS_PER_PAGE) - 1`).
-    With ROWS_PER_PAGE=10, a CSV with 10 rows ends up on a new page 2
-    showing only the new row, so `tbody tr` count drops from 10 → 1.
-    Verifying via REST after save sidesteps the pagination DOM trap.
+    """Add a row, fill the first column + the Comment column, save.
+
+    Two non-obvious traps documented here because both blocked this test
+    on every playwright version until 2026-05-24:
+
+    1) PAGINATION TRAP: after `add_row`, wl_table.js:776 jumps to the
+       new last page. The new row is the LAST visible row on that page
+       (not the first). Calling `set_cell_value(0, ...)` would fill an
+       existing row instead, and the empty new row gets filtered out by
+       wl_save.js:505 (`filter` keeps rows where SOME visible col is
+       non-empty). Net effect on the bad path: 1 silent edit + 1 dropped
+       row = no row growth.
+
+    2) COMMENT-COLUMN GATE: `getAuditComment` (wl_save.js:410) does
+       NOT show an audit-comment modal when the CSV has a `Comment`
+       column. Instead it scans EVERY row's Comment cell; if ANY cell
+       is empty (including the new row we just added), the entire save
+       aborts client-side with "Comment field cannot be empty." — NO
+       save_csv POST is sent.
+
+    Fix: fill the LAST visible row's col 0 AND its Comment cell. The
+    Comment column index varies per CSV; we find it dynamically by
+    looking at the `data-header` attribute of each cell in the new row.
     """
     bak = setup_clean(rest_client)
     initial_total = len(bak.get("rows", []))
@@ -111,9 +117,25 @@ def test_add_row(browser, rest_client):
         page.add_row()
         time.sleep(0.5)
 
-        # The new row is the only one visible on the current (last)
-        # page; index 0 IS the new row.
-        page.set_cell_value(0, 0, new_value)
+        # The new row is the LAST visible row on the current (last) page.
+        visible_after = page.get_row_count()
+        assert visible_after > 0, "Expected at least one visible row after add_row"
+        new_row_idx = visible_after - 1
+
+        # Fill col 0
+        page.set_cell_value(new_row_idx, 0, new_value)
+
+        # Find the Comment col index in the new row by data-header attr
+        new_row = page.page.locator("#csv-table-container table tbody tr").nth(new_row_idx)
+        cells = new_row.locator("td textarea.wl-input, td input.wl-input")
+        comment_idx = None
+        for j in range(cells.count()):
+            if cells.nth(j).get_attribute("data-header") == "Comment":
+                comment_idx = j
+                break
+        if comment_idx is not None:
+            page.set_cell_value(new_row_idx, comment_idx, f"E2E add row test row note")
+
         page.save_changes(comment="E2E add row test")
 
         # REST verification: total row count grew by 1, new value present
@@ -227,17 +249,13 @@ def test_horizontal_scroll_wide_csv(browser, rest_client):
 
 @pytest.mark.crud
 @pytest.mark.e2e
-@pytest.mark.skip(reason=(
-    "Depends on add_row+save persisting a new row, which is the same path that "
-    "test_add_row hits and fails on. Skip linked to test_add_row's skip reason — "
-    "fix both together in the dedicated investigation session."
-))
 def test_crud_workflow_end_to_end(browser, rest_client):
     """End-to-end: load, edit, add, save, verify all via REST.
 
     Edit happens on visible page 1; add_row jumps to last page (per
-    wl_table.js pagination semantics). After add, the new row is at
-    visible index 0 (the only row on the new page).
+    wl_table.js pagination semantics). The new row is the LAST visible
+    row on the new last page (see test_add_row docstring for the
+    pagination-trap explanation).
     """
     bak = setup_clean(rest_client)
     initial_total = len(bak.get("rows", []))
@@ -252,10 +270,21 @@ def test_crud_workflow_end_to_end(browser, rest_client):
         assert page.set_cell_value(0, 0, edit_value)
 
         # Add a row; this jumps to last page where the new row is the
-        # only visible one (visible index 0).
+        # LAST visible one. Fill col 0 AND Comment (see test_add_row
+        # docstring for why Comment is required).
         page.add_row()
         time.sleep(0.5)
-        page.set_cell_value(0, 0, add_value)
+        visible_after_add = page.get_row_count()
+        assert visible_after_add > 0, "Expected at least one visible row after add_row"
+        new_row_idx = visible_after_add - 1
+        page.set_cell_value(new_row_idx, 0, add_value)
+        # Fill Comment col by data-header lookup
+        new_row = page.page.locator("#csv-table-container table tbody tr").nth(new_row_idx)
+        cells = new_row.locator("td textarea.wl-input, td input.wl-input")
+        for j in range(cells.count()):
+            if cells.nth(j).get_attribute("data-header") == "Comment":
+                page.set_cell_value(new_row_idx, j, "E2E e2e row note")
+                break
 
         # Single save commits both changes
         page.save_changes(comment="E2E e2e: edit + add + save")

tests/e2e/test_wl_save.py

@@ -318,12 +318,20 @@ def test_undo_restores_row(self, page: Page):
 class TestBulkRemoval:
 
     def test_bulk_remove(self, page: Page):
+        """Bulk-remove 2 rows, save, verify via REST.
+
+        NOTE: the old assertion `final < initial` used DOM tbody tr
+        count, which is paginated and unreliable. After bulk remove +
+        save + reloadCsvQuiet, if total rows > ROWS_PER_PAGE, page 0
+        still shows ROWS_PER_PAGE rows (so DOM count stays at 10 even
+        though we removed 2). Verify via REST instead.
+        """
         bak = _backup()
+        initial_total = len(bak.get("rows", []))
+        if initial_total < 2:
+            pytest.skip("Need >= 2 rows in CSV")
         try:
             nav_to_csv(page)
-            initial = page.locator("#csv-table-container tbody tr").count()
-            if initial < 2:
-                pytest.skip("Need >= 2 rows")
 
             cbs = page.locator("#csv-table-container tbody input.wl-row-check")
             cbs.nth(0).check()
@@ -343,8 +351,9 @@ def test_bulk_remove(self, page: Page):
                     modal.locator(".btn-danger, .btn-primary").first.click()
                     page.wait_for_timeout(3000)
 
-                    final = page.locator("#csv-table-container tbody tr").count()
-                    assert final < initial, f"Bulk remove failed: {initial} -> {final}"
+                    after = _rest_get("get_csv_content", {"csv_file": TEST_CSV, "app": TEST_APP})
+                    assert len(after.get("rows", [])) < initial_total, \
+                        f"REST row count: {initial_total} -> {len(after.get('rows', []))}, expected decrease"
         finally:
             _restore(bak)
 
@@ -484,25 +493,35 @@ def test_comment_column_skips_modal(self, page: Page):
 class TestAddRowSave:
 
     def test_add_row_and_save(self, page: Page):
+        """Add a row, fill all visible cols, save. Verify via REST.
+
+        NOTE: the old assertion `new_count == initial + 1` was wrong on
+        a paginated table. `add_row` jumps currentPage to the new last
+        page (wl_table.js:776), where the visible row count is just the
+        last-page slice — not initial+1. Verify via REST instead, where
+        we see the true total row count.
+        """
         bak = _backup()
+        initial_total = len(bak.get("rows", []))
         try:
             nav_to_csv(page)
-            initial = page.locator("#csv-table-container tbody tr").count()
 
             page.locator("#btn-add-row").click()
             page.wait_for_timeout(500)
 
-            new_count = page.locator("#csv-table-container tbody tr").count()
-            assert new_count == initial + 1, f"Add row: {initial} -> {new_count}"
-
-            # Fill the new row (last row)
+            # The new row is the LAST visible row on the new last page.
             last_row = page.locator("#csv-table-container tbody tr").last
             textareas = last_row.locator("textarea.wl-input")
             for i in range(textareas.count()):
                 ta = textareas.nth(i)
                 hdr = ta.get_attribute("data-header") or ""
                 if hdr.startswith("_"):
                     continue
+                # Expires uses a date-picker overlay and the underlying
+                # input is read-only — skip it. Comment must be non-empty
+                # (wl_save.js:410 getAuditComment will abort otherwise).
+                if hdr == "Expires":
+                    continue
                 if hdr == "Comment":
                     ta.fill("E2E new row")
                 elif hdr == "user":
@@ -516,12 +535,11 @@ def test_add_row_and_save(self, page: Page):
             page.locator("#btn-save").click()
             page.wait_for_timeout(4000)
 
-            msg = page.locator("#message-container")
-            msg_text = msg.inner_text()
-            msg_class = msg.get_attribute("class") or ""
-            assert "success" in msg_class or "Saved" in msg_text or "Added" in msg_text, \
-                f"Expected save success, got: {msg_text}"
-
+            # Verify via REST: total row count grew by 1 (DOM is paginated
+            # and unreliable for total-row assertions).
+            after = _rest_get("get_csv_content", {"csv_file": TEST_CSV, "app": TEST_APP})
+            assert len(after.get("rows", [])) == initial_total + 1, \
+                f"REST row count: {initial_total} -> {len(after.get('rows', []))}, expected +1"
         finally:
             _restore(bak)