Open the UI to fault testing, and stop mutating string literals

.ci/js-mutation-scope-baseline.json

@@ -357,13 +357,9 @@
     "app/ui/src/hooks/useDebouncedValue.js",
     "app/ui/src/hooks/useDocsUrl.js",
     "app/ui/src/hooks/useElapsedTimer.js",
-    "app/ui/src/hooks/useEntityPage.js",
     "app/ui/src/hooks/useExpandableGraph.js",
     "app/ui/src/hooks/useFeatures.js",
     "app/ui/src/hooks/useFetch.js",
-    "app/ui/src/hooks/useMatrix.js",
-    "app/ui/src/hooks/useMatrixRowOrder.js",
-    "app/ui/src/hooks/useNestedGroupExpand.js",
     "app/ui/src/hooks/usePermissions.js",
     "app/ui/src/hooks/usePersistedState.js",
     "app/ui/src/hooks/useRecentChanges.js",

.github/workflows/js-mutation.yml

@@ -19,7 +19,7 @@
 # time; that is the deliberate trade for the runtime, same as ps-mutation.yml.
 # test/ci-scripts/test-gate-wiring.sh enforces this for every *-mutation.yml.
 #
-# SCOPE IS SMALL AND THAT IS THE POINT OF PUBLISHING IT. Twenty API/UI files are
+# SCOPE IS SMALL AND THAT IS THE POINT OF PUBLISHING IT. 24 API/UI files are
 # mutation-tested out of ~410 eligible; the remaining backlog is counted in
 # .ci/js-mutation-scope-baseline.json and guarded by app/api/src/mutationScope.
 # guard.test.js, which fails when new code is added without a decision. Publishing
@@ -59,7 +59,17 @@ jobs:
   mutation:
     name: 'Mutation: ${{ matrix.scope.label }}'
     runs-on: ubuntu-latest
-    timeout-minutes: 45
+    # 60. The UI scopes are an order of magnitude more expensive per mutant than
+    # the API ones -- every mutant re-renders React under jsdom -- but no scope is
+    # near this now: measured locally, the slowest is 14 minutes (UI list hook,
+    # 402 mutants) against ~5 for the 1,122-mutant API matrix scope. This was
+    # briefly 120, when the UI hooks lived in one config and took 58 minutes; the
+    # fix was splitting that config rather than buying it more time, because a
+    # mutant costs one run of EVERY test in its include set. Keep the margin for
+    # slower CI hardware and for tests yet to be written -- better-covered code is
+    # slower to mutate -- but treat a scope approaching this as a signal to split
+    # it, not to raise the number.
+    timeout-minutes: 60
     strategy:
       # A scope that lands below its floor must not cancel the other three. The
       # point of the weekly run is a complete picture; losing three numbers to one
@@ -71,7 +81,9 @@ jobs:
           - { pkg: api, name: effectiveaccess, label: 'API effective access' }
           - { pkg: api, name: accountlinking,  label: 'API account linking' }
           - { pkg: api, name: matrix,          label: 'API matrix' }
-          - { pkg: ui,  name: pilot,           label: 'UI' }
+          - { pkg: ui,  name: pilot,           label: 'UI permissions + filter' }
+          - { pkg: ui,  name: hooks,           label: 'UI matrix hooks' }
+          - { pkg: ui,  name: listhooks,       label: 'UI list-page hook' }
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
         with:

app/api/stryker.matrix.config.json

@@ -1,6 +1,6 @@
 {
   "$schema": "./node_modules/@stryker-mutator/core/schema/stryker-schema.json",
-  "_comment": "The matrix layer -- the code that decides which access appears in the matrix once inheritance, context rollups and attribute cuts are applied. Chosen as the next scope because it is the closest analogue of the two worst surprises measured so far: src/effectiveAccess/engine.js read 93% line coverage and scored 69% under mutation, src/accountlinking/classifier.js read 97% and scored 68%. Six of these eight files sit at 100% line coverage with their own dedicated test file, which is the same profile, and inheritedAccess.js decides whether a grant is shown as held directly or through a group -- a wrong answer there is silent in both directions and is exactly what a reviewer reads off the matrix. Run: cd app/api && npm run test:mutation:matrix. TEST INCLUDE IS WIDER THAN src/matrix -- see vitest.stryker.matrix.config.js for why: most of these modules are also driven through the route layer, and excluding those tests would manufacture false survivors. MUTATOR SET: everything enabled -- deliberately NOT copying the auth config's StringLiteral/ObjectLiteral exclusions, and the code decided it rather than the assumption. These are SQL builders and app/api/CLAUDE.md notes the unit mocks are SQL-blind, which suggested mutated SQL text would be unkillable; but the tests here assert on the emitted SQL directly (`expect(sql).toContain(\"COUNT(*) FILTER (WHERE t.governed)...\")`), so a mutated literal changes text a test is pinning and dies. That is the opposite of permissions.js, where the strings were prose and 68 of 72 mutants survived. Carry the caveat with the number: pinning SQL text proves it is UNCHANGED, not that the query returns the right rows. Query correctness is the contract tests' job and a high score here does not cover it. MEASURED 63.44% AT A SUITE LINE COVERAGE OF 94.3% -- the widest coverage-to-fault-detection gap recorded in this repo, and the reason this scope was picked. rollupBuilders.js is the sharpest case: 100% of lines, 100% of branches and 100% of functions, and 38% of injected faults still go unnoticed. THE INCLUDE SET IS NOT THE CAUSE: running the full API suite with coverage limited to src/matrix reproduces these per-file figures exactly (94.26% lines / 76.24% branch, same uncovered lines), so the 103 no-coverage mutants are real gaps, not tests this config left out. FLOOR ratchets up only -- 61 -> 65 -> 70 -> 78 as the scope went 63.44 -> 67.02 -> 72.91 -> 80.93. Never lower it to make a red run green. rollupBuilders.js finishes at 96.67 with two survivors that are PROVABLY EQUIVALENT rather than unexamined: both are the ' AND ' separator in `where.join(' AND ')` inside buildRolesAsRowsSql and buildRolesDrillSql, and each of those has exactly one `where.push` in its body, so the list can never hold two conditions and `[x].join(sep) === x` for every sep. They are left visible instead of silenced: the floor sits below them, so they cost nothing, and a future second condition would make them killable again -- which a disable comment would hide. scopeHistory.js followed, 51.66 -> 94.31. Remaining targets, both still on their first measurement: contextRollup.js (68.80) and attributeCut.js (70.93).",
+  "_comment": "The matrix layer -- the code that decides which access appears in the matrix once inheritance, context rollups and attribute cuts are applied. Chosen as the next scope because it is the closest analogue of the two worst surprises measured so far: src/effectiveAccess/engine.js read 93% line coverage and scored 69% under mutation, src/accountlinking/classifier.js read 97% and scored 68%. Six of these eight files sit at 100% line coverage with their own dedicated test file, which is the same profile, and inheritedAccess.js decides whether a grant is shown as held directly or through a group -- a wrong answer there is silent in both directions and is exactly what a reviewer reads off the matrix. Run: cd app/api && npm run test:mutation:matrix. TEST INCLUDE IS WIDER THAN src/matrix -- see vitest.stryker.matrix.config.js for why: most of these modules are also driven through the route layer, and excluding those tests would manufacture false survivors. MUTATOR SET: StringLiteral and ObjectLiteral excluded, matching every other API config. They were enabled for the first measurements and did pay here -- these tests assert on the emitted SQL, so a mutated literal changes text a test pins and dies, which is the opposite of permissions.js where the strings were prose and 68 of 72 survived. But that yield is mostly one-off, while the cost recurs on every weekly run: string/object mutants were 291 of 1,122 here, 26% of the work. The assertions written to kill them stay and keep earning; only the weekly re-measuring of them stops. Note the score RISES on exclusion, 80.93 -> 83.79, because those mutants were killed at a lower rate than the rest -- read the floor below as measured over a smaller mutant set, not as an improvement. Two files move most: contextRollup.js 68.80 -> 82.76 and attributeCut.js 70.93 -> 74.24, which says most of what was unkilled in them was text. 83.79 is a re-run, not arithmetic on the old report: deriving it by dropping string mutants from the previous report gave 83.39, and up to 3 points out per file, so excluding a mutator does not simply subtract its mutants. Carry this caveat regardless: pinning SQL text proves it is unchanged, not that the query returns the right rows. Query correctness is the contract tests' job. MEASURED 63.44% AT A SUITE LINE COVERAGE OF 94.3% -- the widest coverage-to-fault-detection gap recorded in this repo, and the reason this scope was picked. rollupBuilders.js is the sharpest case: 100% of lines, 100% of branches and 100% of functions, and 38% of injected faults still go unnoticed. THE INCLUDE SET IS NOT THE CAUSE: running the full API suite with coverage limited to src/matrix reproduces these per-file figures exactly (94.26% lines / 76.24% branch, same uncovered lines), so the 103 no-coverage mutants are real gaps, not tests this config left out. FLOOR ratchets up only -- 61 -> 65 -> 70 -> 78 as the scope went 63.44 -> 67.02 -> 72.91 -> 80.93 with string mutation on, then 81 against the 83.79 the same tests score with it off. Never lower it to make a red run green. rollupBuilders.js finishes at 96.67 with two survivors that are PROVABLY EQUIVALENT rather than unexamined: both are the ' AND ' separator in `where.join(' AND ')` inside buildRolesAsRowsSql and buildRolesDrillSql, and each of those has exactly one `where.push` in its body, so the list can never hold two conditions and `[x].join(sep) === x` for every sep. They are left visible instead of silenced: the floor sits below them, so they cost nothing, and a future second condition would make them killable again -- which a disable comment would hide. scopeHistory.js followed, 51.66 -> 94.31. Remaining targets, both still on their first measurement: contextRollup.js (68.80) and attributeCut.js (70.93).",
   "packageManager": "npm",
   "testRunner": "vitest",
   "vitest": {
@@ -23,10 +23,16 @@
     "src/matrix/rollupBuilders.js",
     "src/matrix/scopeHistory.js"
   ],
+  "mutator": {
+    "excludedMutations": [
+      "StringLiteral",
+      "ObjectLiteral"
+    ]
+  },
   "thresholds": {
     "high": 90,
     "low": 70,
-    "break": 78
+    "break": 81
   },
   "concurrency": 4,
   "timeoutMS": 60000,

app/ui/package.json

@@ -17,8 +17,10 @@
     "test:e2e:sql": "bash scripts/e2e-sql.sh",
     "test:e2e:headed": "npx playwright test --headed",
     "test:e2e:ui": "npx playwright test --ui",
-    "test:mutation": "npm run test:mutation:pilot",
-    "test:mutation:pilot": "stryker run stryker.pilot.config.json"
+    "test:mutation": "npm run test:mutation:pilot && npm run test:mutation:hooks && npm run test:mutation:listhooks",
+    "test:mutation:pilot": "stryker run stryker.pilot.config.json",
+    "test:mutation:hooks": "stryker run stryker.hooks.config.json",
+    "test:mutation:listhooks": "stryker run stryker.listhooks.config.json"
   },
   "dependencies": {
     "@azure/msal-browser": "^5.18.0",

app/ui/src/hooks/useEntityPage.test.jsx

@@ -407,3 +407,77 @@ describe('useEntityPage', () => {
     expect(groups.result.current.search).toBe('');
   });
 });
+
+// ── When the sidecar fetches fail ───────────────────────────────────────────
+// Columns and tags are loaded alongside the list, and each has the same two-step
+// guard: `res.ok ? res.json() : null`, then `if (data) setX(data)`. Neither
+// failure arm was reachable from this file -- only a mount test elsewhere in the
+// suite happened to drive them, which is a poor place for the contract to live
+// and left these mutants unkilled here.
+//
+// What must NOT happen is the interesting part: a failed columns fetch must leave
+// the previous columns alone rather than blanking them, because the filter bar is
+// built from that list. Overwriting it with null or [] silently removes every
+// filter the user could apply, and an empty filter bar looks like a page with
+// nothing to filter on rather than a page whose request failed.
+describe('useEntityPage sidecar failures', () => {
+  // KEY ORDER MATTERS: makeAuthFetch matches by substring, and the columns URL
+  // ('/api/users/columns') contains the list URL ('/api/users'). List-first would
+  // answer the columns request with the list payload -- an object where an array
+  // is expected, which fails deep inside getFilterFields rather than at the mock.
+  const listOnly = (extra = {}) => ({
+    ...extra,
+    [LIST]: { data: [{ id: '1', displayName: 'Bob' }], total: 1 },
+  });
+
+  it('keeps the list usable when the columns request fails', async () => {
+    const { result } = setup({
+      handler: listOnly({
+        [COLUMNS]: jsonResponse({ error: 'nope' }, { ok: false, status: 500 }),
+        '/api/tags': [{ id: 't1', name: 'VIP' }],
+      }),
+    });
+
+    await waitFor(() => expect(result.current.loading).toBe(false));
+    // The rows still arrive — one failed sidecar does not take the page down.
+    expect(result.current.items).toEqual([{ id: '1', displayName: 'Bob' }]);
+    // ...and the loading flag still settles, or the filter bar spins forever.
+    await waitFor(() => expect(result.current.columnsLoading).toBe(false));
+    // Tags are unaffected: the two fetches fail independently.
+    expect(result.current.tags).toEqual([{ id: 't1', name: 'VIP' }]);
+  });
+
+  it('keeps the list usable when the tags request fails', async () => {
+    const { result } = setup({
+      handler: listOnly({
+        [COLUMNS]: [{ column: 'department', values: ['Sales'] }],
+        '/api/tags': jsonResponse({ error: 'nope' }, { ok: false, status: 503 }),
+      }),
+    });
+
+    await waitFor(() => expect(result.current.loading).toBe(false));
+    expect(result.current.items).toEqual([{ id: '1', displayName: 'Bob' }]);
+    // Columns are unaffected — again, independent failures. Asserted through
+    // getOptionsForField rather than getFilterFields: the latter dereferences the
+    // `fieldLabels` prop with no default, so it throws for any caller that omits
+    // one. Worth knowing, but not this test's subject.
+    await waitFor(() => expect(result.current.columnsLoading).toBe(false));
+    expect(result.current.getOptionsForField('department')).toEqual(['Sales']);
+  });
+
+  it('leaves both sidecars at their defaults when both fail', async () => {
+    const { result } = setup({
+      handler: listOnly({
+        [COLUMNS]: jsonResponse({ error: 'nope' }, { ok: false, status: 500 }),
+        '/api/tags': jsonResponse({ error: 'nope' }, { ok: false, status: 500 }),
+      }),
+    });
+
+    await waitFor(() => expect(result.current.loading).toBe(false));
+    await waitFor(() => expect(result.current.columnsLoading).toBe(false));
+    // Both stay empty rather than becoming null: every consumer maps over them.
+    expect(result.current.tags).toEqual([]);
+    expect(result.current.getOptionsForField('department')).toEqual([]);
+    expect(result.current.items).toEqual([{ id: '1', displayName: 'Bob' }]);
+  });
+});