fix(dynamic): keep groupmembership stereotype wrappers when normalizing rulesets

docs/handbuch/dynamic-groups.md

@@ -6,7 +6,7 @@ sources:
   - src/engine/dynamic.ts
   - src/engine/synthetic.ts
   - src/application/operations/adopt-group.ts
-sources_hash: e38b8c0f6032d5cc
+sources_hash: 3872161fc3777e2b
 reviewed: 2026-08-28
 ---
 
@@ -382,15 +382,16 @@ sibling of `query`, not an argument to `churchQuery(...)`.
 
 `q` (`src/config/query.ts`) emits a JSONLogic tree:
 
-| helper                     | emits                                   |
-| -------------------------- | --------------------------------------- |
-| `q.and(...nodes)`          | `{ and: nodes }`                        |
-| `q.or(...nodes)`           | `{ or: nodes }`                         |
-| `q.not(node)`              | `{ "!": [node] }`                       |
-| `q.var(name)`              | `{ var: name }`                         |
-| `q.eq(varName, value)`     | `{ "==": [{ var: varName }, value] }`   |
-| `q.oneof(varName, values)` | `{ oneof: [{ var: varName }, values] }` |
-| `q.isnull(varName)`        | `{ isnull: [{ var: varName }] }`        |
+| helper                     | emits                                                    |
+| -------------------------- | -------------------------------------------------------- |
+| `q.and(...nodes)`          | `{ and: nodes }`                                         |
+| `q.or(...nodes)`           | `{ or: nodes }`                                          |
+| `q.not(node)`              | `{ "!": [node] }`                                        |
+| `q.var(name)`              | `{ var: name }`                                          |
+| `q.eq(varName, value)`     | `{ "==": [{ var: varName }, value] }`                    |
+| `q.oneof(varName, values)` | `{ oneof: [{ var: varName }, values] }`                  |
+| `q.isnull(varName)`        | `{ isnull: [{ var: varName }] }`                         |
+| `q.memberOf(node)`         | `{ dterm: [{ stereotype: ["groupmembership"] }, node] }` |
 
 `var` values may be **logical references** or **raw ids**. Prefer a reference so
 the ruleset is portable across hosts (#20): `q.eq("ctgroup.campusId",
@@ -439,8 +440,51 @@ const ruleset = {
 ChurchTools' stored rulesets carry cosmetic noise that would otherwise show
 up as permanent phantom diffs on every `ct plan`:
 
-- **`dterm: [label, expr]` wrappers** — a cosmetic UI label around a
-  subtree. Never evaluated by ChurchTools; stripped down to `expr`.
+- **`dterm: [label, expr]` wrappers** — a UI label around a subtree. A string
+  or `{ title }` label is cosmetic and stripped down to `expr`. A label that
+  carries a **`stereotype`** (`{ stereotype: ["groupmembership"], title }`) is
+  **not** cosmetic and is kept: it makes ChurchTools evaluate the wrapped group
+  condition per person ("is / is not a member of group X") rather than per
+  membership row. Without it, a negated group condition such as
+  `!(member of "Unsubscribed")` still matches everyone who has any other
+  membership, so it excludes no one. When you author a group condition —
+  especially a negated one — wrap it the way the ChurchTools editor does. With
+  the typed builder that is `q.memberOf`:
+
+  ```ts
+  q.not(q.memberOf(q.oneof("ctgroup.id", [ref.group("unsubscribed")])));
+  ```
+
+  and in a JSON ruleset file:
+
+  ```json
+  {
+    "!": [
+      {
+        "dterm": [
+          { "stereotype": ["groupmembership"], "title": "Mitgliedschaft in einer Gruppe" },
+          {
+            "oneof": [{ "var": "ctgroup.id" }, [{ "__ctRef": true, "kind": "group", "key": "unsubscribed" }]]
+          }
+        ]
+      }
+    ]
+  }
+  ```
+
+  Up to v4.1.0 the normalizer stripped these wrappers too, and because `apply`
+  writes the normalized ruleset, applying a ruleset silently removed them. Two
+  consequences when you upgrade:
+
+  - Rulesets captured by `ct adopt` before the fix lack the wrappers. `ct plan`
+    now warns (`applying would remove N "stereotype" dterm wrapper(s)`) for each
+    group whose live ruleset still has them. Re-adopt those rulesets before you
+    apply.
+  - A group that an earlier `apply` already stripped on the server has no
+    wrapper left to re-adopt, and `ct plan` shows nothing. Add the wrapper back
+    by hand (above), or re-save the condition in the ChurchTools editor and
+    re-adopt.
+
 - **int/string inconsistency** — the same logical id shows up as both `1`
   and `"1"` across (and even within) a single ruleset. Numeric-looking
   strings inside the `query` subtree are coerced to numbers.

docs/handbuch/group-member-fields.md

@@ -1,5 +1,5 @@
 ---
-sources_hash: 802017b4678885e0
+sources_hash: ab2983f780a57d6d
 title: Group member fields
 sources:
   - src/engine/member-fields.ts

src/config/query.ts

@@ -28,6 +28,10 @@ export const q = {
   eq: (varName: string, value: unknown): QueryNode => ({ "==": [{ var: varName }, value] }),
   oneof: (varName: string, values: unknown[]): QueryNode => ({ oneof: [{ var: varName }, values] }),
   isnull: (varName: string): QueryNode => ({ isnull: [{ var: varName }] }),
+  /** Evaluate a group condition per person ("is a member of …"), not per membership row — wrap
+   *  every `ctgroup.*` condition you negate: `q.not(q.oneof("ctgroup.id", …))` alone excludes no
+   *  one who has any other membership. Same `groupmembership` stereotype the CT editor writes. */
+  memberOf: (n: QueryNode): QueryNode => ({ dterm: [{ stereotype: ["groupmembership"] }, n] }),
 };
 
 /** Wrap a JSONLogic filter in the ChurchQuery envelope the ruleset `query` field expects.

src/engine/dynamic.ts

@@ -13,13 +13,40 @@ import type { DynamicStatus } from "./types.js";
 
 const READ_ONLY_KEYS = new Set(["dynamicGroupUpdateStarted", "dynamicGroupUpdateFinished"]);
 
-/** Recursively unwrap `dterm: [label, expr]` cosmetic wrappers to their `expr`. */
+/**
+ * Whether a `dterm` label is pure display text: a string, or an object carrying nothing but a
+ * `title`. Everything else is kept. In particular a `stereotype` label (in practice
+ * `["groupmembership"]`) is NOT cosmetic: it tells ChurchTools to evaluate the wrapped group
+ * condition per person ("is/is not a member of group X") instead of per membership row. Without
+ * it, `!(member of X)` is true for every person who also has any other membership row, so a
+ * negated group condition excludes no one. Measured read-only against 72 live rulesets on CT 3.137
+ * (`POST /churchquery/debug/export`): keeping these labels reproduces every live result exactly;
+ * dropping them changes 18 of them. An allowlist, so a label key ChurchTools adds later is kept
+ * rather than silently stripped.
+ */
+export function isCosmeticLabel(label: unknown): boolean {
+  if (label === null || label === undefined || typeof label === "string") return true;
+  if (typeof label !== "object" || Array.isArray(label)) return false;
+  return Object.keys(label).every((k) => k === "title");
+}
+
+function isLabelWrapper(obj: Record<string, unknown>): obj is { dterm: [unknown, unknown] } {
+  return Array.isArray(obj.dterm) && obj.dterm.length === 2 && Object.keys(obj).length === 1;
+}
+
+/**
+ * Recursively unwrap cosmetic `dterm: [label, expr]` wrappers to their `expr`. A wrapper whose
+ * label is not cosmetic (see `isCosmeticLabel`) changes what the query selects and is kept, its
+ * label copied verbatim and its `expr` normalized in place.
+ */
 export function stripCosmeticLabels(node: unknown): unknown {
   if (Array.isArray(node)) return node.map(stripCosmeticLabels);
   if (node && typeof node === "object") {
     const obj = node as Record<string, unknown>;
-    if (Array.isArray(obj.dterm) && obj.dterm.length === 2 && Object.keys(obj).length === 1) {
-      return stripCosmeticLabels(obj.dterm[1]); // keep the expression, drop the label
+    if (isLabelWrapper(obj)) {
+      const [label, expr] = obj.dterm;
+      if (!isCosmeticLabel(label)) return { dterm: [structuredClone(label), stripCosmeticLabels(expr)] };
+      return stripCosmeticLabels(expr); // keep the expression, drop the label
     }
     const out: Record<string, unknown> = {};
     for (const [k, v] of Object.entries(obj)) out[k] = stripCosmeticLabels(v);
@@ -28,6 +55,17 @@ export function stripCosmeticLabels(node: unknown): unknown {
   return node;
 }
 
+/** The non-cosmetic `dterm` labels in a (normalized) query, in document order. */
+export function semanticLabels(node: unknown): unknown[] {
+  if (Array.isArray(node)) return node.flatMap(semanticLabels);
+  if (node && typeof node === "object") {
+    const obj = node as Record<string, unknown>;
+    const own = isLabelWrapper(obj) && !isCosmeticLabel(obj.dterm[0]) ? [obj.dterm[0]] : [];
+    return [...own, ...Object.values(obj).flatMap(semanticLabels)];
+  }
+  return [];
+}
+
 /**
  * Coerce numeric-string leaves to numbers (CT is int/string-inconsistent for `var` values).
  *
@@ -41,8 +79,12 @@ export function stripCosmeticLabels(node: unknown): unknown {
 export function coerceScalars(node: unknown): unknown {
   if (Array.isArray(node)) return node.map(coerceScalars);
   if (node && typeof node === "object") {
+    const obj = node as Record<string, unknown>;
+    // A kept `dterm` label is free text written back to ChurchTools verbatim: a title "2024" must
+    // stay a string. Only the wrapped expression carries the int/string-inconsistent ids.
+    if (isLabelWrapper(obj)) return { dterm: [structuredClone(obj.dterm[0]), coerceScalars(obj.dterm[1])] };
     const out: Record<string, unknown> = {};
-    for (const [k, v] of Object.entries(node as Record<string, unknown>)) out[k] = coerceScalars(v);
+    for (const [k, v] of Object.entries(obj)) out[k] = coerceScalars(v);
     return out;
   }
   if (typeof node === "string" && /^(-?[1-9]\d*|0)$/.test(node)) {

src/engine/synthetic.ts

@@ -15,7 +15,14 @@ import { deepEqual } from "./plan.js";
 import { mapConcurrent } from "../util/concurrency.js";
 import { info, warn, formatError } from "../ui.js";
 import { slug } from "../resources/registry.js";
-import { normalizeDynamic, normalizeRuleset, putRulesetBody, resolveRulesetRef } from "./dynamic.js";
+import {
+  normalizeDynamic,
+  normalizeRuleset,
+  putRulesetBody,
+  resolveRulesetRef,
+  semanticLabels,
+  type NormalizedDynamic,
+} from "./dynamic.js";
 import { formatPortablizeWarnings, scanUnportablized } from "../config/query-refs.js";
 import {
   actualMemberFieldProps,
@@ -638,6 +645,19 @@ const dynamicField: SyntheticField = {
         for (const line of details) info(`    ${line}`);
       }
       const dynamic = normalizeDynamic({ status: d.dynamic.status, ruleset: resolvedRuleset });
+      // A ruleset adopted while the normalizer still stripped `stereotype` labels lacks them, and
+      // applying it would PUT the stripped query back — which turns a negated group condition into
+      // one that excludes no one. Say so before apply does it; re-adopting restores the wrappers.
+      const live = (actual.get(d.key)?.dynamic as NormalizedDynamic | undefined)?.ruleset;
+      const dropped = semanticLabels(live?.query).length - semanticLabels(dynamic.ruleset.query).length;
+      if (dropped > 0) {
+        const headline =
+          `dynamic group "${d.key}": applying would remove ${dropped} "stereotype" dterm wrapper(s) ` +
+          `the live ruleset has — a negated group condition without one excludes no one. ` +
+          `Re-adopt the ruleset (or add the wrapper) before applying.`;
+        warnings.push(headline);
+        warn(headline);
+      }
       return { ...d, fields: { ...d.fields, dynamic } };
     });
     return { desired: augmented, errors, unreadable, ...(warnings.length > 0 ? { warnings } : {}) };

tests/dynamic.test.ts

@@ -6,25 +6,51 @@ import {
   coerceScalars,
   normalizeDynamic,
   putRulesetBody,
+  semanticLabels,
 } from "../src/engine/dynamic.js";
 
 describe("stripCosmeticLabels", () => {
   it("unwraps a dterm with a string label to its expr, recursively", () => {
     const input = { and: [{ dterm: ["Campus", { "==": [{ var: "ctgroup.campusId" }, 1] }] }] };
     expect(stripCosmeticLabels(input)).toEqual({ and: [{ "==": [{ var: "ctgroup.campusId" }, 1] }] });
   });
-  it("unwraps a dterm with an object label (title/stereotype/i18n key)", () => {
+  it("unwraps a dterm with a title-only object label (i18n key)", () => {
     const input = {
-      dterm: [
-        { title: "group.x.title", stereotype: ["groupmembership"] },
-        { isnull: [{ var: "person.dateOfDeath" }] },
-      ],
+      dterm: [{ title: "group.x.title" }, { isnull: [{ var: "person.dateOfDeath" }] }],
     };
     expect(stripCosmeticLabels(input)).toEqual({ isnull: [{ var: "person.dateOfDeath" }] });
   });
+  it("keeps a groupmembership stereotype wrapper: it changes what a negated group condition selects", () => {
+    // `!(member of X)` excludes no one once apply has written it without this wrapper — the label
+    // is part of the query, not decoration.
+    const label = { stereotype: ["groupmembership"], title: "Mitgliedschaft in einer Gruppe" };
+    const input = {
+      "!": [
+        { dterm: [label, { and: [{ dterm: ["Merkmal", { oneof: [{ var: "ctgroup.id" }, ["42"]] }] }] }] },
+      ],
+    };
+    const out = stripCosmeticLabels(input);
+    expect(out).toEqual({
+      "!": [{ dterm: [label, { and: [{ oneof: [{ var: "ctgroup.id" }, ["42"]] }] }] }],
+    });
+    expect(stripCosmeticLabels(out)).toEqual(out); // idempotent
+    expect((out as { "!": [{ dterm: [unknown] }] })["!"][0].dterm[0]).not.toBe(label); // copied
+  });
+  it("keeps a label with any key besides title: unknown label keys are not assumed cosmetic", () => {
+    const input = { dterm: [{ title: "t", hint: "perperson" }, { isnull: [{ var: "person.dateOfDeath" }] }] };
+    expect(stripCosmeticLabels(input)).toEqual(input);
+  });
 });
 
 describe("coerceScalars", () => {
+  it("leaves a kept dterm label verbatim: a numeric-looking title is free text, not an id", () => {
+    const input = {
+      dterm: [{ stereotype: ["groupmembership"], title: "2024" }, { oneof: [{ var: "ctgroup.id" }, ["5"]] }],
+    };
+    expect(coerceScalars(input)).toEqual({
+      dterm: [{ stereotype: ["groupmembership"], title: "2024" }, { oneof: [{ var: "ctgroup.id" }, [5]] }],
+    });
+  });
   it("coerces numeric strings to numbers so int/string drift is not spurious", () => {
     expect(coerceScalars({ "==": [{ var: "ctgroup.campusId" }, "1"] })).toEqual({
       "==": [{ var: "ctgroup.campusId" }, 1],
@@ -95,12 +121,17 @@ describe("normalizeRuleset", () => {
     expect(JSON.stringify(normalizeRuleset(once))).toBe(JSON.stringify(once)); // byte-identical + idempotent
   });
 
-  it("read-then-normalize of every live fixture is stable and label-free", () => {
+  it("read-then-normalize of every live fixture is stable and keeps only stereotype labels", () => {
     for (const name of ["ruleset-683", "ruleset-2022", "ruleset-1092"]) {
       const raw = JSON.parse(readFileSync(`tests/fixtures/dynamic/${name}.get.json`, "utf8")); // array shape
       const once = normalizeRuleset(raw);
       expect(normalizeRuleset(once)).toEqual(once); // idempotent
-      expect(JSON.stringify(once)).not.toContain("dterm"); // cosmetic labels stripped
+      // cosmetic labels stripped, every stereotype wrapper the fixture carries survives
+      const wrappers = (JSON.stringify(once).match(/"dterm"/g) ?? []).length;
+      const stereotypes = (JSON.stringify(raw).match(/"stereotype"/g) ?? []).length;
+      expect(stereotypes).toBeGreaterThan(0);
+      expect(semanticLabels(once.query)).toHaveLength(stereotypes);
+      expect(wrappers).toBe(stereotypes);
     }
   });
 });

tests/query.test.ts

@@ -1,6 +1,6 @@
 import { describe, it, expect } from "vitest";
 import { q, churchQuery } from "../src/config/query.js";
-import { normalizeRuleset } from "../src/engine/dynamic.js";
+import { normalizeRuleset, stripCosmeticLabels } from "../src/engine/dynamic.js";
 
 describe("typed query builder", () => {
   it("builds a JSONLogic tree", () => {
@@ -22,6 +22,14 @@ describe("typed query builder", () => {
     expect(q.not(operand)).toEqual({ "!": [{ isnull: [{ var: "person.dateOfDeath" }] }] });
   });
 
+  it("q.memberOf wraps in the groupmembership stereotype, which normalization keeps", () => {
+    const node = q.memberOf(q.oneof("ctgroup.id", [7]));
+    expect(node).toEqual({
+      dterm: [{ stereotype: ["groupmembership"] }, { oneof: [{ var: "ctgroup.id" }, [7]] }],
+    });
+    expect(stripCosmeticLabels(node)).toEqual(node);
+  });
+
   it("q.or", () => {
     const a = q.eq("ctgroup.campusId", 1);
     const b = q.eq("person.isArchived", false);

tests/synthetic-dynamic.test.ts

@@ -533,3 +533,61 @@ describe("dynamic synthetic field — un-portablized ruleset reporting (#101)",
     expect(errs.join("")).not.toContain("host-specific");
   });
 });
+
+describe("dynamic synthetic field — dropped stereotype wrappers", () => {
+  /**
+   * A ruleset adopted while the normalizer still stripped `stereotype` labels lacks them. Applying
+   * it would PUT the stripped query over a live one that has them, so a negated group condition
+   * silently stops excluding anyone. Plan time is where the author can still re-adopt.
+   */
+  const membership = (expr: unknown) => ({ dterm: [{ stereotype: ["groupmembership"], title: "t" }, expr] });
+  const negated = { oneof: [{ var: "ctgroup.id" }, [7]] };
+  const state: State = {
+    version: 1,
+    host: "h",
+    resources: {
+      g: { type: "group", id: 5, key: "g", fields: { name: "G" }, adoptedAt: "t", updatedAt: "t" },
+    },
+  };
+  const fold = async (live: unknown, declared: unknown) => {
+    const actual = new Map<string, Record<string, unknown>>([["g", { name: "G" }]]);
+    const desired: DesiredResource[] = [
+      {
+        type: "group",
+        key: "g",
+        fields: { name: "G" },
+        dependsOn: [],
+        dynamic: { status: "active", ruleset: declared },
+      },
+    ];
+    const client = {
+      get: vi.fn(async (p: string) => (p.endsWith("/ruleset") ? live : { dynamicGroupStatus: "active" })),
+    };
+    const errs: string[] = [];
+    const spy = vi.spyOn(process.stderr, "write").mockImplementation((s) => {
+      errs.push(String(s));
+      return true;
+    });
+    try {
+      const result = await dynamicField().fold({ client: getClient(client), state, desired, actual });
+      return { result, out: errs.join("") };
+    } finally {
+      spy.mockRestore();
+    }
+  };
+
+  it("warns when the declared ruleset lacks a wrapper the live one has", async () => {
+    const { result, out } = await fold([{ query: { "!": [membership(negated)] } }], {
+      query: { "!": [negated] },
+    });
+    expect(out).toContain('dynamic group "g": applying would remove 1 "stereotype" dterm wrapper(s)');
+    expect(result?.warnings?.join("\n")).toContain("Re-adopt the ruleset");
+  });
+
+  it("stays silent when both sides carry the wrapper", async () => {
+    const ruleset = { query: { "!": [membership(negated)] } };
+    const { result, out } = await fold([ruleset], ruleset);
+    expect(out).not.toContain("stereotype");
+    expect(result?.warnings?.join("\n") ?? "").not.toContain("stereotype");
+  });
+});