fix: "has an answer" / "is not selected" rules reopen in the inline builder (#15)

client/src/ui/CalculationBuilder.tsx

@@ -1342,6 +1342,9 @@ function conditionProse(cond: ParsedExpression): string {
     if (r.kind === 'answered') {
       return `\${${r.field}} ${r.negated ? 'is empty' : 'is answered'}`;
     }
+    if (r.kind === 'truthy') {
+      return `\${${r.field}} ${r.negated ? 'is not selected' : 'has an answer'}`;
+    }
     if (r.kind === 'age') {
       return `age of \${${r.field}} ${r.op} ${r.value} years`;
     }

client/src/ui/DecisionsView.tsx

@@ -487,6 +487,8 @@ function relevantToHumanLines(cond: import('@cht-ui/shared').ParsedExpression):
         return `${r.negated ? 'NOT ' : ''}${r.field} includes "${r.value}"`;
       case 'answered':
         return r.negated ? `${r.field} is empty` : `${r.field} is answered`;
+      case 'truthy':
+        return r.negated ? `${r.field} is not selected` : `${r.field} has an answer`;
       case 'age':
         return `age of ${r.field} ${r.op} ${r.value} years`;
       case 'date_offset':

client/src/ui/RelevantRuleBuilder.tsx

@@ -483,6 +483,28 @@ function RuleRow(props: {
       </div>
     );
   }
+  if (rule.kind === 'truthy') {
+    // `${f}` / `not(${f})` — what the inline builder writes for "has an
+    // answer" / "is not selected" (T9b, #15). Same labels here so a rule
+    // reads the same in both builders.
+    return (
+      <div className="row gap rule-row">
+        <FieldPicker
+          value={rule.field}
+          options={props.fieldOptions}
+          onChange={(v) => props.onChange({ ...rule, field: v })}
+        />
+        <select
+          value={rule.negated ? 'not' : 'ref'}
+          onChange={(e) => props.onChange({ ...rule, negated: e.target.value === 'not' })}
+        >
+          <option value="ref">has an answer</option>
+          <option value="not">is not selected</option>
+        </select>
+        <button className="link danger" onClick={props.onRemove}>×</button>
+      </div>
+    );
+  }
   if (rule.kind === 'contact-input-comparison') {
     // Mirrors the ComparisonRule shape but with a datalist on the LHS
     // backed by the project's contact-input field list ∪ the CHT-canonical

client/tests/reopen-truthy-answered.spec.ts

@@ -0,0 +1,174 @@
+/**
+ * T9b (#15) — rules the inline builder writes for "has an answer" (`${f}`)
+ * and "is not selected" (`not(${f})`) must reopen in the inline builder,
+ * and `${f} != ''` (already in the fixture as `lmp_note`'s relevant) must
+ * open as a clause and save back byte-identical.
+ *
+ * "Reopens as a clause" is observable in the strip as: no "hand-written"
+ * status, the `↶ undo last clause` control present (it only renders when a
+ * clause is committed), and `+ insert` enabled. A hand-written rule shows
+ * the status line and disables both.
+ *
+ * Runs on a throwaway copy of `fixtures/mini-config` so the committed
+ * fixture never changes. Bytes are asserted through the API (`GET
+ * /api/forms/app:pregnancy`), i.e. what the server re-parsed from the
+ * sheet on disk after the UI saved it.
+ */
+import { test, expect } from './setup.js';
+import type { Locator, Page } from '@playwright/test';
+import { promises as fs } from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const here = path.dirname(fileURLToPath(import.meta.url));
+const FIXTURE_DIR = path.resolve(here, 'fixtures', 'mini-config');
+const API = 'http://127.0.0.1:5174';
+
+type SurveyRow = { name?: string; type: string; extras?: Record<string, string> };
+
+function rowByType(page: Page, rawType: RegExp): Locator {
+  return page
+    .locator('.survey-row')
+    .filter({ has: page.locator('code.type-chip-raw', { hasText: rawType }) });
+}
+
+async function openPregnancy(page: Page): Promise<void> {
+  await page.goto('/');
+  await page.locator('.nav-item', { hasText: 'Forms' }).click();
+  await page.getByRole('button', { name: 'pregnancy.xlsx' }).click();
+  await expect(page.locator('.survey-row').first()).toBeVisible();
+}
+
+async function saveForm(page: Page): Promise<void> {
+  await page.locator('.page-header').getByRole('button', { name: 'Save', exact: true }).click();
+  await page.locator('.rule-builder-card').getByRole('button', { name: 'Save', exact: true }).click();
+  await expect(
+    page.locator('.page-header').getByRole('button', { name: 'Saved', exact: true }),
+  ).toBeVisible();
+}
+
+/** Open the row's advanced panel and point the unified strip at `column`. */
+async function openStrip(row: Locator, column: 'relevant' | 'constraint'): Promise<Locator> {
+  await row.getByRole('button', { name: /show advanced/ }).click();
+  const strip = row.locator('.cond-strip-unified');
+  await expect(strip).toBeVisible();
+  await strip.locator('.ref-chip-select').nth(0).selectOption(column);
+  return strip;
+}
+
+/** The raw column textbox for `column` inside the row's advanced panel. */
+function rawColumnInput(row: Locator, column: string): Locator {
+  return row
+    .locator('label.expr-field')
+    .filter({ has: row.page().locator('code.raw-col-tag', { hasText: new RegExp(`^${column}$`) }) })
+    .locator('input')
+    .first();
+}
+
+async function expectOpenAsClause(strip: Locator): Promise<void> {
+  await expect(strip.getByText(/This rule was hand-written/)).toHaveCount(0);
+  await expect(strip.getByRole('button', { name: '↶ undo last clause' })).toBeVisible();
+  await expect(strip.getByRole('button', { name: '+ insert' })).toBeEnabled();
+  await expect(strip.locator('.ref-chip-select').nth(1)).toBeEnabled();
+}
+
+test('T9b — "has an answer" and "is not selected" written by the strip reopen as clauses with controls enabled', async ({
+  page,
+  request,
+}) => {
+  const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'cht-ui-t9b-'));
+  try {
+    await fs.cp(FIXTURE_DIR, tmp, { recursive: true });
+    expect((await request.post(`${API}/api/project/open`, { data: { path: tmp } })).ok()).toBeTruthy();
+    await openPregnancy(page);
+
+    // gravidity (integer): relevant = "lmp_date has an answer" → `${lmp_date}`.
+    const gravidity = rowByType(page, /^integer$/);
+    let strip = await openStrip(gravidity, 'relevant');
+    await strip.locator('.ref-chip-select').nth(1).selectOption('lmp_date');
+    await strip.locator('.ref-chip-select').nth(2).selectOption('ref');
+    await strip.getByRole('button', { name: '+ insert' }).click();
+    await expect(rawColumnInput(gravidity, 'relevant')).toHaveValue('${lmp_date}');
+    // The strip re-hydrates from what it just wrote: a clause, not hand-written text.
+    await expectOpenAsClause(strip);
+
+    // chair_rise (select_one): relevant = "danger_signs is not selected" → `not(${danger_signs})`.
+    const chairRise = rowByType(page, /^select_one pass_fail$/);
+    strip = await openStrip(chairRise, 'relevant');
+    await strip.locator('.ref-chip-select').nth(1).selectOption('danger_signs');
+    await strip.locator('.ref-chip-select').nth(2).selectOption('not');
+    await strip.getByRole('button', { name: '+ insert' }).click();
+    await expect(rawColumnInput(chairRise, 'relevant')).toHaveValue('not(${danger_signs})');
+    await expectOpenAsClause(strip);
+
+    await saveForm(page);
+
+    // Bytes on disk are exactly what the builder has always written.
+    const saved = (await (await request.get(`${API}/api/forms/app:pregnancy`)).json()) as {
+      form: { survey: SurveyRow[] };
+    };
+    const byName = new Map(saved.form.survey.map((r) => [r.name, r]));
+    expect(byName.get('gravidity')?.extras?.relevant).toBe('${lmp_date}');
+    expect(byName.get('chair_rise')?.extras?.relevant).toBe('not(${danger_signs})');
+
+    // Cold reopen after a reload: both rows come back as clauses, not text.
+    await openPregnancy(page);
+    strip = await openStrip(rowByType(page, /^integer$/), 'relevant');
+    await expectOpenAsClause(strip);
+    strip = await openStrip(rowByType(page, /^select_one pass_fail$/), 'relevant');
+    await expectOpenAsClause(strip);
+  } finally {
+    await request.post(`${API}/api/project/open`, { data: { path: FIXTURE_DIR } });
+    await fs.rm(tmp, { recursive: true, force: true });
+  }
+});
+
+test("T9b — an existing `${f} != ''` relevant opens as a clause and saves back byte-identical", async ({
+  page,
+  request,
+}) => {
+  const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'cht-ui-t9b-'));
+  try {
+    await fs.cp(FIXTURE_DIR, tmp, { recursive: true });
+    expect((await request.post(`${API}/api/project/open`, { data: { path: tmp } })).ok()).toBeTruthy();
+
+    const before = (await (await request.get(`${API}/api/forms/app:pregnancy`)).json()) as {
+      form: { survey: SurveyRow[] };
+    };
+    expect(before.form.survey.find((r) => r.name === 'lmp_note')?.extras?.relevant).toBe(
+      "${lmp_date} != ''",
+    );
+
+    await openPregnancy(page);
+    const lmpNote = rowByType(page, /^note$/);
+    const strip = await openStrip(lmpNote, 'relevant');
+    await expectOpenAsClause(strip);
+
+    // Re-insert with zero edits: the clause is written back as it was
+    // spelled, never normalised to `${lmp_date}`.
+    await strip.getByRole('button', { name: '+ insert' }).click();
+    await expect(rawColumnInput(lmpNote, 'relevant')).toHaveValue("${lmp_date} != ''");
+
+    // Dirty the form elsewhere so Save is reachable, then save.
+    const gravidity = rowByType(page, /^integer$/);
+    const gStrip = await openStrip(gravidity, 'relevant');
+    await gStrip.locator('.ref-chip-select').nth(1).selectOption('lmp_date');
+    await gStrip.locator('.ref-chip-select').nth(2).selectOption('ref');
+    await gStrip.getByRole('button', { name: '+ insert' }).click();
+    await saveForm(page);
+
+    const after = (await (await request.get(`${API}/api/forms/app:pregnancy`)).json()) as {
+      form: { survey: SurveyRow[] };
+    };
+    expect(after.form.survey.find((r) => r.name === 'lmp_note')?.extras?.relevant).toBe(
+      "${lmp_date} != ''",
+    );
+    expect(after.form.survey.find((r) => r.name === 'gravidity')?.extras?.relevant).toBe(
+      '${lmp_date}',
+    );
+  } finally {
+    await request.post(`${API}/api/project/open`, { data: { path: FIXTURE_DIR } });
+    await fs.rm(tmp, { recursive: true, force: true });
+  }
+});

shared/src/conditionBuilder/conditionReducer.test.ts

@@ -820,3 +820,108 @@ test('relabel guard: every ClauseOp value is one of the canonical 11 (no label l
 test('FieldKind co-domain mirror stays in sync with classifier', () => {
   assert.equal(ALL_FIELD_KINDS_INC_UNKNOWN.length, 6);
 });
+
+/* ------------------- T9b (#15): reopen "has an answer" / "is not selected" ------------------- */
+/*
+ * Two distinct defects met at `ruleToClause`, so they are pinned separately.
+ * A bare `${f}` fixture exercises the (formerly raw) truthy path and never
+ * the `answered` path; `${f} != ''` is the reverse. One test cannot cover
+ * both — the assertions on the parser kind below make that explicit.
+ */
+
+test('T9b case (a): `${field}` written by the inline builder reopens as op=ref with chaining enabled', () => {
+  // The builder itself emits this exact text for "has an answer".
+  let written = withColumn('relevant');
+  written = setDraft(written, { field: 'lmp_approx', op: 'ref', value: '' });
+  const bytes = serializeBuilderState(written);
+  assert.equal(bytes, '${lmp_approx}');
+  // Document which parser path this fixture exercises (NOT `answered`).
+  assert.equal(parseRelevant(bytes).rules[0]?.kind, 'truthy');
+
+  const reopened = conditionBuilderReducer(initialConditionBuilderState, {
+    kind: 'set-column',
+    column: 'relevant',
+    existingValue: bytes,
+  });
+  assert.equal(reopened.rawFallback, null);
+  assert.deepEqual(reopened.clauses, [{ field: 'lmp_approx', op: 'ref', value: '' }]);
+  assert.equal(serializeBuilderState(reopened), bytes);
+});
+
+test('T9b case (a): `not(${field})` written by the inline builder reopens as op=not, and chains', () => {
+  let written = withColumn('relevant');
+  written = setDraft(written, { field: 'danger_signs', op: 'not', value: '' });
+  written = commit(written, 'and');
+  written = setDraft(written, { field: 'age', op: '>', value: '18' });
+  const bytes = serializeBuilderState(written);
+  assert.equal(bytes, 'not(${danger_signs}) and ${age} > 18');
+
+  const reopened = conditionBuilderReducer(initialConditionBuilderState, {
+    kind: 'set-column',
+    column: 'relevant',
+    existingValue: bytes,
+  });
+  assert.equal(reopened.rawFallback, null);
+  assert.deepEqual(reopened.clauses, [
+    { field: 'danger_signs', op: 'not', value: '' },
+    { field: 'age', op: '>', value: '18' },
+  ]);
+  assert.equal(reopened.lockedConnector, 'and');
+  assert.equal(serializeBuilderState(reopened), bytes);
+});
+
+test('T9b case (a): a spacing-divergent `${ field }` is still raw (self-check stays authoritative)', () => {
+  const existing = '${ lmp_approx }';
+  const s = conditionBuilderReducer(initialConditionBuilderState, {
+    kind: 'set-column',
+    column: 'relevant',
+    existingValue: existing,
+  });
+  assert.equal(s.rawFallback, existing);
+  assert.equal(s.clauses.length, 0);
+});
+
+test("T9b case (b): `${field} != ''` (parser kind `answered`) reopens as \"has an answer\" and is re-emitted unchanged", () => {
+  const existing = "${lmp_date} != ''";
+  // Document which parser path this fixture exercises (NOT `truthy`).
+  assert.equal(parseRelevant(existing).rules[0]?.kind, 'answered');
+
+  const s = conditionBuilderReducer(initialConditionBuilderState, {
+    kind: 'set-column',
+    column: 'relevant',
+    existingValue: existing,
+  });
+  assert.equal(s.rawFallback, null);
+  assert.equal(s.clauses.length, 1);
+  assert.equal(s.clauses[0]?.field, 'lmp_date');
+  assert.equal(s.clauses[0]?.op, 'ref');
+  assert.equal(s.clauses[0]?.value, '');
+  // Invariant 1 — never normalised to `${lmp_date}`.
+  assert.equal(serializeBuilderState(s), existing);
+});
+
+test("T9b case (b): `${field} = ''` reopens as \"is not selected\" and chains without rewriting the spelling", () => {
+  const existing = "${lmp_date} = '' and ${sex} = 'female'";
+  const s = conditionBuilderReducer(initialConditionBuilderState, {
+    kind: 'set-column',
+    column: 'relevant',
+    existingValue: existing,
+  });
+  assert.equal(s.rawFallback, null);
+  assert.equal(s.clauses.length, 2);
+  assert.equal(s.clauses[0]?.op, 'not');
+  assert.deepEqual(s.clauses[1], { field: 'sex', op: '=', value: 'female' });
+  assert.equal(serializeBuilderState(s), existing);
+
+  // Appending a clause keeps the hydrated spelling and emits the new one canonically.
+  const extended = commit(setDraft(s, { field: 'age', op: '>', value: '18' }), 'and');
+  assert.equal(serializeBuilderState(extended), `${existing} and \${age} > 18`);
+});
+
+test('T9b case (b): a stale `source` that no longer describes the clause is ignored, not emitted', () => {
+  // Only reachable by a caller constructing state by hand — the reducer never
+  // edits a committed clause — but the guard is what makes `source` safe.
+  let s = withColumn('relevant');
+  s = setDraft(s, { field: 'other', op: 'ref', value: '', source: "${lmp_date} != ''" });
+  assert.equal(serializeBuilderState(s), '${other}');
+});