fix(engagement): the six defects the browser pass found (Phase 4b)

Driving the two screens in Chrome, after the API walk had already found the two
in Phase 4a's code. None of these is visible from a test or from curl.

Two cost an operator something real:

  - The Audience dropdown rendered EMPTY before a trigger was chosen. There is
    genuinely nothing it may offer without a ceiling, but a select with zero
    options reads as broken rather than as waiting. It now says "Choose a
    trigger first..." and is disabled.
  - A `members` audience with no saved audience reaches NOBODY, and only the
    preview button said so. That is the design, but it is also the default the
    instant a members-ceiling trigger is picked - so the rule saves, gets
    switched on, and mails nobody with nothing on screen saying so. The editor
    now says it inline, and stands down once a preview has answered the same
    question more precisely.

One the server was already refusing, just too late:

  - The composer offered "exclude" on the only row, building an `and` whose
    every child is a complement. The server refuses it correctly but only after
    a save, and it is one checkbox away at all times. Now refused inline, in the
    operator's words.

Three wording and layout:

  - the template-key input truncated its placeholder, and said "optional until
    Phase 5" - a sentence about the plan document, not about the deployment
  - "segment" leaked into a screen that says "saved audience" everywhere else.
    The API, schema and docs keep saying segment (one word for one table);
    translated at the point of display only
  - the composer repeated its AUDIENCE heading above every row

Client only - no server change, so swagger and the route manifest are untouched.
Client suite 316/316; all six verified in the browser after the fix.

- [x] AI-assisted: written with Claude Code (Opus)

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

client/src/lib/engagementRules.js

@@ -101,15 +101,56 @@ export function segmentChoicesFor(trigger, ceilings, segments) {
  */
 export function describeReach(preview) {
   if (!preview) return ''
-  if (preview.dormant) return `Resolves to nobody right now — ${preview.reason || 'dormant'}.`
+  const why = operatorWords(preview.reason)
+  if (preview.dormant) return `Resolves to nobody right now — ${why || 'dormant'}.`
   if (preview.permitted === false) {
     return `Reaches ${preview.count}, but this trigger does not permit that audience — saving will be refused.`
   }
-  if (preview.reason) return `${preview.count} right now — ${preview.reason}.`
+  if (why) return `${preview.count} right now — ${why}.`
   if (preview.capped) return `At least ${preview.count} people (the preview stops counting there).`
   return preview.count === 1 ? '1 person right now.' : `${preview.count} people right now.`
 }
 
+/**
+ * The server says "segment"; these screens say "saved audience".
+ *
+ * The API, the schema and the docs all call it a segment and should keep doing
+ * so - it is one word for one table. But an operator meets the concept here,
+ * under a heading that says "Audiences", and a sentence that switches vocabulary
+ * mid-screen reads as a sentence about something else.
+ */
+export function operatorWords(text) {
+  if (!text) return text
+  // Word-wise rather than a regex, so "segmented" and the like are left alone.
+  const swap = { segment: 'saved audience', segments: 'saved audiences' }
+  return String(text)
+    .split(' ')
+    .map((word) => swap[word] || word)
+    .join(' ')
+}
+
+/**
+ * The one audience choice that silently reaches nobody, said out loud.
+ *
+ * `members` is the ceiling for "a module-declared list". Without a saved
+ * audience naming WHICH list there is no list, and core knows no game vocabulary
+ * with which to guess - so the rule resolves to the empty set every time it
+ * fires. It is also the DEFAULT the moment an operator picks a `members`-ceiling
+ * trigger, which is what makes it a trap rather than a curiosity: the rule saves,
+ * switches on, and mails nobody, with nothing on the screen saying so unless the
+ * operator happens to press Preview.
+ *
+ * Returns a sentence, or null when there is nothing to warn about.
+ */
+export function audienceWarning(form) {
+  if (!form) return null
+  if (form.audienceSegmentId) return null
+  if (form.audience === 'members') {
+    return 'This reaches nobody as it stands. “Members of a module-declared list” needs a saved audience naming which list.'
+  }
+  return null
+}
+
 // ── Segment expressions ────────────────────────────────────────────────────
 
 /**
@@ -127,7 +168,16 @@ export function notPlacementError(expression) {
     if (!node || typeof node !== 'object') return null
     if (!node.op) return null
     if (node.op === 'not' && !underAnd) {
-      return 'A "not" can only be used inside an "all of" group — on its own it would mean "everyone except…".'
+      return 'An excluded audience can only be used alongside an included one — on its own it would mean “everyone except…”.'
+    }
+    // The same rule from the other side: a group of nothing but exclusions has
+    // no set to take them from. The composer offers "exclude" on every row, so
+    // this is one checkbox away at all times and is worth saying before the
+    // round trip - the server refuses it, correctly, but only after a save.
+    if ((node.op === 'and' || node.op === 'or') && (node.nodes || []).length) {
+      if ((node.nodes || []).every((c) => c && c.op === 'not')) {
+        return 'At least one audience has to be included — a list made only of exclusions has nothing to exclude from.'
+      }
     }
     for (const child of node.nodes || []) {
       const err = walk(child, node.op === 'and')

client/src/routes/admin/views/EngagementAudiences.jsx

@@ -50,12 +50,13 @@ const toGroup = (expression) =>
 
 // ── One leaf: an audience and its declared parameters ──────────────────────
 
-function LeafRow({ audiences, node, onChange, onRemove, negated, onToggleNegate, canNegate }) {
+function LeafRow({ audiences, node, onChange, onRemove, negated, onToggleNegate, canNegate, first }) {
   const declared = audiences.find((a) => a.id === node.audienceId)
   return (
     <div style={{ display: 'flex', gap: 8, marginBottom: 8, flexWrap: 'wrap', alignItems: 'flex-end' }}>
       <label style={{ flex: '1 1 240px' }}>
-        <span className="field-label">Audience</span>
+        {/* The heading belongs to the group, not to every line in it. */}
+        {first && <span className="field-label">Audience</span>}
         <select
           className="select"
           value={node.audienceId || ''}
@@ -198,6 +199,7 @@ function SegmentEditor({ audiences, segment, onSaved, onCancel }) {
           return (
             <LeafRow
               key={i}
+              first={i === 0}
               audiences={audiences}
               node={leaf}
               negated={negated}

client/src/routes/admin/views/EngagementRules.jsx

@@ -8,6 +8,7 @@ import {
   segmentChoicesFor,
   describeReach,
   describeRule,
+  audienceWarning,
   conditionRowsFrom,
   conditionsFromRows,
   operatorsForType,
@@ -201,9 +202,14 @@ function RuleEditor({ catalog, segments, rule, onSaved, onCancel }) {
           <select
             className="select"
             value={form.audienceSegmentId ? '' : form.audience}
-            disabled={Boolean(form.audienceSegmentId)}
+            disabled={Boolean(form.audienceSegmentId) || !audienceChoices.length}
             onChange={(e) => { set({ audience: e.target.value, audienceSegmentId: null }); setPreview(null) }}
           >
+            {/* Without a trigger there is no ceiling, so there is nothing this
+                may legitimately offer — and a select with zero options renders
+                as a control that is broken rather than as one that is waiting. */}
+            {!audienceChoices.length && <option value="">Choose a trigger first…</option>}
+            {Boolean(form.audienceSegmentId) && <option value="">Using the saved audience →</option>}
             {audienceChoices.map((c) => (
               <option key={c.id} value={c.id}>{c.label}</option>
             ))}
@@ -241,6 +247,15 @@ function RuleEditor({ catalog, segments, rule, onSaved, onCancel }) {
           {describeReach(preview)}
         </p>
       )}
+      {/* The `members`-with-no-saved-audience trap, said before the save rather
+          than discovered after it. It is the DEFAULT the moment a
+          members-ceiling trigger is chosen, and the rule it produces saves,
+          switches on and mails nobody. */}
+      {!preview && audienceWarning(form) && (
+        <p className="sans" style={{ margin: '10px 0 0', fontSize: '0.84rem', color: 'var(--accent)' }}>
+          {audienceWarning(form)}
+        </p>
+      )}
       {trigger && audienceChoices.length <= 1 && (
         <p className="sans" style={{ margin: '10px 0 0', fontSize: '0.8rem', color: 'var(--muted)' }}>
           This event only permits “{trigger.ceiling}”. The audience a rule may use is capped by the
@@ -260,8 +275,8 @@ function RuleEditor({ catalog, segments, rule, onSaved, onCancel }) {
             {form.channels.includes(c.id) && (
               <input
                 className="input"
-                style={{ marginTop: 6 }}
-                placeholder="template key (optional until Phase 5)"
+                style={{ marginTop: 6, width: '100%' }}
+                placeholder="template key (optional)"
                 value={form.templateKeys[c.id] || ''}
                 onChange={(e) => set({ templateKeys: { ...form.templateKeys, [c.id]: e.target.value } })}
               />

client/test/engagementRules.test.js

@@ -9,6 +9,8 @@ import {
   describeRule,
   describeExpression,
   notPlacementError,
+  audienceWarning,
+  operatorWords,
   conditionRowsFrom,
   conditionsFromRows,
   operatorsForType,
@@ -218,6 +220,34 @@ test('a single stored comparison is one editable row', () => {
 
 // ── Segment composition ────────────────────────────────────────────────────
 
+test('a members audience with no saved audience is warned about BEFORE the save', () => {
+  // The trap the browser walk found: it is the default the moment a
+  // members-ceiling trigger is chosen, and the rule it produces saves, switches
+  // on and mails nobody. Nothing on the screen said so unless you pressed
+  // Preview.
+  assert.match(audienceWarning({ audience: 'members', audienceSegmentId: null }), /reaches nobody/)
+  assert.equal(audienceWarning({ audience: 'members', audienceSegmentId: 4 }), null)
+  assert.equal(audienceWarning({ audience: 'owner', audienceSegmentId: null }), null)
+})
+
+test('the server says "segment"; the screens say "saved audience"', () => {
+  // One word for one table in the API, the schema and the docs. But an operator
+  // meets the concept under a heading that says "Audiences", and a sentence that
+  // switches vocabulary mid-screen reads as being about something else.
+  assert.equal(operatorWords('audience segment is dormant'), 'audience saved audience is dormant')
+  assert.match(describeReach({ count: 0, dormant: true, reason: 'audience segment is dormant' }), /saved audience/)
+  // and it does not maul a word that merely contains it
+  assert.equal(operatorWords('segmented data'), 'segmented data')
+})
+
+test('a list of nothing but exclusions is refused before the round trip', () => {
+  // One checkbox away at all times, because the composer offers "exclude" on
+  // every row including the only one. The server refuses it correctly — but
+  // only after a save.
+  const err = notPlacementError({ op: 'and', nodes: [{ op: 'not', nodes: [{ audienceId: 'a' }] }] })
+  assert.match(err, /at least one audience/i)
+})
+
 test('a bare not is refused before it reaches the server', () => {
   assert.ok(notPlacementError({ op: 'not', nodes: [{ audienceId: 'uo.governors' }] }))
   assert.ok(notPlacementError({ op: 'or', nodes: [{ audienceId: 'a' }, { op: 'not', nodes: [{ audienceId: 'b' }] }] }))