feat(engagement): Admin → Engagement → Rules and Audiences (engagement Phase 4b) #171

Merged
whitlocktech merged 2 commits from feature/engagement-rules-admin into edge 2026-08-29 17:28:56 +00:00
Member

Engagement Phase 4b — the admin surface over the Phase 4a engine. Companion docs PR: docs#184.

Two screens, twelve routes and a reach preview. Nothing in the engine changed; what changed is that an operator can reach it, which is what turns 4a's model into rules a person will actually meet. enabled still defaults to 0 and core still seeds no rules, so this PR does not start any mail — it makes starting some possible.

Four decisions settled before any code

Question Decision
Segments a fifth nav entry, a sub-tab, or inline in the rule editor its own nav entry, "Audiences" — a segment is reusable, independently edited, and deleted by a refusal that has to say how many rules still point at it. None of that has anywhere to live inside a screen about rules
The switch PATCH …/enabled, or a full PUT its own route, writing that column and no other
Preview count only, count plus a sample, or none in 4b count only, on demand
Delete hard delete with a confirm, or disable-only hard delete — the send log survives it

Q4's answer from Phase 4a lands here too: Engagement is its own top-level nav group, beside Content / Moderation / System, with Rules and Audiences in it now and Triggers / Templates / the send log joining in Phase 5. Email Delivery stays a section of Settings.

The enable switch is a correctness argument, not a convenience

A PUT re-validates against the registries as they are now. So the rules a re-validating toggle cannot switch off are exactly three: a rule whose module has been uninstalled, one naming a channel that is gone, and one whose trigger has since narrowed its ceiling underneath a saved audience. Those are the three rules an operator most urgently wants stopped — a toggle built on PUT is broken in precisely the case it is needed.

PATCH /admin/engagement/rules/:id/enabled writes one column and always works. Switching a rule on without re-validation is safe for a different reason: audiences.permitted runs again at send time, so an enabled-but-no-longer-permitted rule resolves to nobody rather than to the wrong people. There is a test for each half — the dormant rule that switches off, and the full update of that same rule being refused.

The reach preview: a count, and three ways it refuses to lie

It calls the engine's own resolver (audiences.resolveForRule) rather than a second query that agrees with it today — a preview built out of its own SQL can be wrong about the one thing it exists to say.

It answers a count and nothing else. Not a sample, not names: the resolver's output for a module-declared segment is a set of players derived from game data, and an editor that rendered them would be a user-enumeration surface reached from a screen about mail scheduling. A test asserts the response carries no identities.

  • capped — every audience query is bounded at 5000, so a count landing on the bound is a floor. The screen says "at least 5000", never "5000".
  • reason — an owner audience resolves per event from an id the event carries, so it has no advance answer. It reports 0 with the reason; a bare 0 reads as "nobody".
  • permitted — whether the trigger's G24 ceiling allows the reach just counted. Without it the editor shows a healthy number beside a save the server will refuse, which reads as a bug in the save rather than as the ceiling doing its job.

Two defects found by walking the API against a live server — both in Phase 4a's code

1. A rule pointing at a dormant segment read as healthy. listAnnotated asked only whether the segment row still existed — §7.3's case, the segment was deleted. The other shape of the same failure is §5.1a rule 4's: the segment row is exactly where it was and every audience in it belongs to a module that has been uninstalled. Both leave the rule reaching nobody; only one leaves nothing behind. Uninstalling a module under an enabled rule produced a rule the new screen showed as on, healthy and firing, which is the one thing this screen exists not to do.

The expression walk now lives in engagement/segments.js as missingAudiences, and both the segment list and the rule list ask it, so the two cannot disagree about what dormant means. Verified live, and pinned by a test that leaves the segment row in place and takes the module away.

2. "1 rule still use this segment". The delete refusal pluralised the noun and not the verb — in the sentence an operator reads at the moment they are being told no.

Six more from driving the screens in a browser

None of these is visible from a test or from curl, and two of them cost an operator something real:

  1. The Audience dropdown rendered empty before a trigger was chosen. audienceChoicesFor(null) answers [] — correctly, because without a trigger there is no ceiling and so nothing it may legitimately offer. But a <select> with zero options reads as a control that is broken, not one that is waiting. It now says "Choose a trigger first…" and is disabled.
  2. A members audience with no saved audience reaches nobody, and only the preview button said so. That is the design (§5.1a) — and it is also the default the instant an operator picks any members-ceiling trigger, which turns a documented property into a trap: the rule saves, gets switched on, and mails nobody, with nothing on screen saying so unless the operator happens to press Preview. Said inline now, before the save, and it stands down once a preview has answered the same question more precisely.
  3. The composer offered "exclude" on the only row, building an and whose every child is a complement. The server refuses that correctly — "has nothing but complements, there is no set to exclude from" — but only after a save, and it is one checkbox away at all times. Refused inline now, in the operator's words.
  4. The template-key input truncated its own placeholder, and said "optional until Phase 5" — a sentence about the plan document rather than about the operator's deployment.
  5. "segment" leaked into a screen that says "saved audience" everywhere else. The API, the schema and the docs keep saying segment (one word for one table); it is translated at the point of display only.
  6. The composer repeated its "AUDIENCE" heading above every row; the heading belongs to the group.

All six verified in the browser after the fix, and each is pinned by a test where there is something to pin.

Three smaller things the screens decided

  • A rule's trigger is fixed once the rule exists. engagementRules.db.update never carried trigger_id; this is where that becomes a stated rule rather than an omission. A rule's cooldown rows, its pending outbox rows and its send-log history are all about one trigger id, and re-pointing the rule silently re-attributes all three. The editor renders the field read-only and says why.
  • A condition tree the editor cannot render is shown, not flattened. The editor offers the flat half of the grammar — one and/or over comparisons, every operator a dropdown narrowed to the picked variable's declared type. A AND (B OR C) flattened to A AND B AND C fires on different events and the operator would have no way to know the save had done it, so such a rule opens read-only with its JSON and one honest choice: leave it, or clear it.
  • Literals are coerced to the declared type before the save. Every value in an HTML input is a string and the server rightly refuses {cmp: 'gt', value: "5"} against an int. A value that does not parse is passed through unchanged rather than becoming NaN or false, so the refusal names the variable instead of the rule saving cleanly having compared against a number nobody typed.

Where the logic lives

client/src/lib/engagementRules.js — plain JS, 28 tests — holds everything the screens decide: the form↔payload mapping, which audiences a trigger permits (derived from the permits list the server serves, never a second copy of the lattice), the reach sentence, the not-placement check, the condition round trip. None of it is a boundary — the server decides and the engine re-checks — so the client holds the affordance (do not offer what will be refused) and the server holds the answer. It is a .js because the test runner cannot reach a .jsx.

Verification

  • 21 new server tests (test/engagementAdmin.test.js) and 28 client tests (client/test/engagementRules.test.js), all green. Client suite 316/316; client build clean.
  • The server suite's single failure — the committed manifest matches the declarations in the tree — is the known Windows CRLF artifact and fails identically on clean edge. routes.manifest and routes.guards were the other two and are green here because this PR regenerates them.
  • Walked end to end against the live local stack, first over the API and then through both screens in a browser: the JSON columns round-tripping through a real MariaDB, the ceiling refusal, the preview against 16 real users and 8 real staff, the 409 with its count, the OR-takes-the-tighter-ceiling arithmetic, members AND NOT staff being allowed, two incomparable ceilings being refused, the dormancy path in both directions, and a rule composed, saved, summarised, switched on and deleted from the screens themselves.
  • Core declares no audiences, so the whole §5.1a half is untestable on a stock stack. A ~20-line throwaway module (modules/rig/, gitignored) declaring four audiences at three ceilings is what made that walk possible; it is recorded in the docs PR for whoever needs it next. Its ids must be namespaced to the module's own id, and must contain a dot.
  • Swagger and the route manifest regenerated (core-only, with modules/uo moved aside).

Two environment facts the browser pass turned up, recorded in the docs PR: rebuilding client/dist while the server is running blanks the whole SPA (the HTML shell resolves core's hashed bundle name at boot, so window.__rg is never published and modules/uo/entry.js throws MODULE_API.md §3.1's error into a blank page — the error names core, and core is not at fault); and a window.confirm blocks CDP entirely, so the two destructive actions cannot be driven from a script.


  • AI-assisted: written with Claude Code (Opus)

🤖 Generated with Claude Code

Engagement **Phase 4b** — the admin surface over the Phase 4a engine. Companion docs PR: **docs#184**. Two screens, twelve routes and a reach preview. Nothing in the engine changed; what changed is that an operator can reach it, which is what turns 4a's model into rules a person will actually meet. `enabled` still defaults to 0 and core still seeds no rules, so this PR does not start any mail — it makes starting some possible. ## Four decisions settled before any code | | Question | Decision | |---|---|---| | **Segments** | a fifth nav entry, a sub-tab, or inline in the rule editor | **its own nav entry, "Audiences"** — a segment is reusable, independently edited, and deleted by a refusal that has to say how many rules still point at it. None of that has anywhere to live inside a screen about rules | | **The switch** | `PATCH …/enabled`, or a full `PUT` | **its own route**, writing that column and no other | | **Preview** | count only, count plus a sample, or none in 4b | **count only, on demand** | | **Delete** | hard delete with a confirm, or disable-only | **hard delete** — the send log survives it | Q4's answer from Phase 4a lands here too: **Engagement is its own top-level nav group**, beside Content / Moderation / System, with Rules and Audiences in it now and Triggers / Templates / the send log joining in Phase 5. Email Delivery stays a section of Settings. ## The enable switch is a correctness argument, not a convenience A `PUT` re-validates against the registries **as they are now**. So the rules a re-validating toggle cannot switch off are exactly three: a rule whose module has been uninstalled, one naming a channel that is gone, and one whose trigger has since **narrowed** its ceiling underneath a saved audience. Those are the three rules an operator most urgently wants stopped — a toggle built on `PUT` is broken in precisely the case it is needed. `PATCH /admin/engagement/rules/:id/enabled` writes one column and always works. Switching a rule *on* without re-validation is safe for a different reason: `audiences.permitted` runs again at send time, so an enabled-but-no-longer-permitted rule resolves to nobody rather than to the wrong people. There is a test for each half — the dormant rule that switches off, and the full update of that same rule being refused. ## The reach preview: a count, and three ways it refuses to lie It calls **the engine's own resolver** (`audiences.resolveForRule`) rather than a second query that agrees with it today — a preview built out of its own SQL can be wrong about the one thing it exists to say. It answers **a count and nothing else**. Not a sample, not names: the resolver's output for a module-declared segment is a set of players derived from game data, and an editor that rendered them would be a user-enumeration surface reached from a screen about mail scheduling. A test asserts the response carries no identities. - **`capped`** — every audience query is bounded at 5000, so a count landing on the bound is a floor. The screen says "at least 5000", never "5000". - **`reason`** — an `owner` audience resolves per event from an id the event carries, so it has no advance answer. It reports 0 *with the reason*; a bare 0 reads as "nobody". - **`permitted`** — whether the trigger's G24 ceiling allows the reach just counted. Without it the editor shows a healthy number beside a save the server will refuse, which reads as a bug in the save rather than as the ceiling doing its job. ## Two defects found by walking the API against a live server — both in Phase 4a's code **1. A rule pointing at a *dormant* segment read as healthy.** `listAnnotated` asked only whether the segment **row** still existed — §7.3's case, the segment was deleted. The other shape of the same failure is §5.1a rule 4's: the segment row is exactly where it was and every audience in it belongs to a module that has been uninstalled. Both leave the rule reaching nobody; only one leaves nothing behind. Uninstalling a module under an enabled rule produced a rule the new screen showed as **on, healthy and firing**, which is the one thing this screen exists not to do. The expression walk now lives in `engagement/segments.js` as `missingAudiences`, and both the segment list and the rule list ask it, so the two cannot disagree about what dormant means. Verified live, and pinned by a test that leaves the segment row in place and takes the module away. **2. `"1 rule still use this segment"`.** The delete refusal pluralised the noun and not the verb — in the sentence an operator reads at the moment they are being told no. ## Six more from driving the screens in a browser None of these is visible from a test or from `curl`, and two of them cost an operator something real: 1. **The Audience dropdown rendered empty before a trigger was chosen.** `audienceChoicesFor(null)` answers `[]` — correctly, because without a trigger there is no ceiling and so nothing it may legitimately offer. But a `<select>` with zero options reads as a control that is *broken*, not one that is waiting. It now says "Choose a trigger first…" and is disabled. 2. **A `members` audience with no saved audience reaches nobody, and only the preview button said so.** That is the design (§5.1a) — and it is also the **default** the instant an operator picks any `members`-ceiling trigger, which turns a documented property into a trap: the rule saves, gets switched on, and mails nobody, with nothing on screen saying so unless the operator happens to press Preview. Said inline now, before the save, and it stands down once a preview has answered the same question more precisely. 3. **The composer offered "exclude" on the only row**, building an `and` whose every child is a complement. The server refuses that correctly — "has nothing but complements, there is no set to exclude from" — but only after a save, and it is one checkbox away at all times. Refused inline now, in the operator's words. 4. **The template-key input truncated its own placeholder**, and said "optional until Phase 5" — a sentence about the plan document rather than about the operator's deployment. 5. **"segment" leaked into a screen that says "saved audience" everywhere else.** The API, the schema and the docs keep saying *segment* (one word for one table); it is translated at the point of display only. 6. The composer repeated its "AUDIENCE" heading above every row; the heading belongs to the group. All six verified in the browser after the fix, and each is pinned by a test where there is something to pin. ## Three smaller things the screens decided - **A rule's trigger is fixed once the rule exists.** `engagementRules.db.update` never carried `trigger_id`; this is where that becomes a stated rule rather than an omission. A rule's cooldown rows, its pending outbox rows and its send-log history are all about one trigger id, and re-pointing the rule silently re-attributes all three. The editor renders the field read-only and says why. - **A condition tree the editor cannot render is shown, not flattened.** The editor offers the flat half of the grammar — one and/or over comparisons, every operator a dropdown narrowed to the picked variable's declared type. `A AND (B OR C)` flattened to `A AND B AND C` fires on different events and the operator would have no way to know the save had done it, so such a rule opens read-only with its JSON and one honest choice: leave it, or clear it. - **Literals are coerced to the declared type before the save.** Every value in an HTML input is a string and the server rightly refuses `{cmp: 'gt', value: "5"}` against an `int`. A value that does not parse is passed through **unchanged** rather than becoming `NaN` or `false`, so the refusal names the variable instead of the rule saving cleanly having compared against a number nobody typed. ## Where the logic lives `client/src/lib/engagementRules.js` — plain JS, 28 tests — holds everything the screens *decide*: the form↔payload mapping, which audiences a trigger permits (derived from the `permits` list the server serves, never a second copy of the lattice), the reach sentence, the `not`-placement check, the condition round trip. **None of it is a boundary** — the server decides and the engine re-checks — so the client holds the affordance (do not offer what will be refused) and the server holds the answer. It is a `.js` because the test runner cannot reach a `.jsx`. ## Verification - **21 new server tests** (`test/engagementAdmin.test.js`) and **28 client tests** (`client/test/engagementRules.test.js`), all green. Client suite 316/316; client build clean. - The server suite's single failure — `the committed manifest matches the declarations in the tree` — is the known Windows CRLF artifact and fails identically on clean `edge`. `routes.manifest` and `routes.guards` were the other two and are green here because this PR regenerates them. - **Walked end to end against the live local stack**, first over the API and then through both screens in a browser: the JSON columns round-tripping through a real MariaDB, the ceiling refusal, the preview against 16 real users and 8 real staff, the `409` with its count, the `OR`-takes-the-tighter-ceiling arithmetic, `members AND NOT staff` being allowed, two incomparable ceilings being refused, the dormancy path in both directions, and a rule composed, saved, summarised, switched on and deleted from the screens themselves. - Core declares **no audiences**, so the whole §5.1a half is untestable on a stock stack. A ~20-line throwaway module (`modules/rig/`, gitignored) declaring four audiences at three ceilings is what made that walk possible; it is recorded in the docs PR for whoever needs it next. Its ids must be **namespaced to the module's own id**, and must contain a dot. - Swagger and the route manifest regenerated (core-only, with `modules/uo` moved aside). Two environment facts the browser pass turned up, recorded in the docs PR: **rebuilding `client/dist` while the server is running blanks the whole SPA** (the HTML shell resolves core's hashed bundle name at boot, so `window.__rg` is never published and `modules/uo/entry.js` throws MODULE_API.md §3.1's error into a blank page — the error names core, and core is not at fault); and **a `window.confirm` blocks CDP entirely**, so the two destructive actions cannot be driven from a script. --- - [x] AI-assisted: written with Claude Code (Opus) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
wtclaude added 1 commit 2026-08-29 17:11:20 +00:00
feat(engagement): Admin - Engagement - Rules and Audiences (engagement Phase 4b)
All checks were successful
PR Checks / bot-tests (pull_request) Successful in 31s
PR Checks / client-build (pull_request) Successful in 32s
PR Checks / server-tests (pull_request) Successful in 10m36s
4b45eddb5d
The admin surface over the Phase 4a engine: two screens, twelve routes and the
reach preview. Nothing in the engine changed; what changed is that an operator
can now reach it.

Four decisions settled by the org lead before any code:

  - segments get their OWN nav entry, "Audiences", not a tab of the rules screen
  - the on/off switch is its own PATCH route, not a full PUT
  - the reach preview is a count only, on demand
  - a rule can be hard-deleted; the send log survives it

The switch is the one with real content in it. A PUT re-validates against the
registries as they are NOW, so the rules a re-validating toggle cannot switch
off are exactly the three an operator most wants stopped: a rule whose module
was uninstalled, one naming a channel that is gone, and one whose trigger has
since narrowed its ceiling under a saved audience. PATCH .../enabled writes one
column and always works. Switching ON unvalidated is safe because the engine
re-checks the ceiling at send time.

The preview calls the engine's own resolver rather than a second query that
agrees with it today, and answers a count and nothing else - the resolver's
output for a module-declared segment is a set of players derived from game data.
It reports `capped` at the 5000-row bound (the count is a floor, not a total),
`reason` for an `owner` audience (which resolves per event and has no advance
answer), and `permitted` so the editor cannot show a healthy number beside a
save the server will refuse.

Two defects found by walking it against a live server, both in Phase 4a's code:

  1. A rule pointing at a DORMANT segment read as healthy. listAnnotated asked
     only whether the segment ROW existed. The other shape of the same failure
     is a segment sitting exactly where it was whose every audience belongs to
     an uninstalled module: same outcome, nothing deleted. Uninstalling a module
     under an enabled rule produced a rule the screen showed as on and firing.
     The expression walk now lives in engagement/segments.js as
     `missingAudiences` and both lists ask it.
  2. "1 rule still use this segment" - the delete refusal pluralised the noun
     and not the verb, in the sentence an operator reads when told no.

Also: a rule's trigger is now a stated rule rather than an omission in the
UPDATE statement (its cooldowns, queued sends and history are all about one
trigger id); a condition tree the editor cannot render is shown read-only rather
than flattened, because flattening changes which events fire the rule; and
literals are coerced client-side to the type the trigger declared, with anything
that does not parse passed through unchanged so the server's refusal names the
variable.

Tests: 21 new server tests (test/engagementAdmin.test.js) and 25 client ones
(client/test/engagementRules.test.js), all green. The single failure in the
server suite (`the committed manifest matches the declarations in the tree`) is
the known Windows CRLF artifact and fails identically on clean edge.

Companion docs PR: docs#184.

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

Co-Authored-By: Claude <noreply@anthropic.com>
wtclaude added 1 commit 2026-08-29 17:26:52 +00:00
fix(engagement): the six defects the browser pass found (Phase 4b)
All checks were successful
PR Checks / client-build (pull_request) Successful in 30s
PR Checks / server-tests (pull_request) Successful in 2m35s
PR Checks / bot-tests (pull_request) Successful in 8m34s
3a7a08425c
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>
whitlocktech merged commit 1d7961e7a2 into edge 2026-08-29 17:28:56 +00:00
whitlocktech deleted branch feature/engagement-rules-admin 2026-08-29 17:28:57 +00:00
Sign in to join this conversation.
No description provided.