From 9e62c6f8f0f60d35438c17f32e04af00bac5fd0d Mon Sep 17 00:00:00 2001 From: Prajwol Shrestha Date: Thu, 1 Oct 2026 17:19:46 +0545 Subject: [PATCH 1/2] fix: rules the inline builder writes for "has an answer" / "is not selected" reopen as clauses (#15) Two defects met at ruleToClause in conditionReducer.ts: (a) clauseToRule emitted `${f}` and `not(${f})` as kind 'raw', so on reopen the parser handed back a raw rule, hydrateColumn fell to rawFallback and every control was disabled. They now have a real parser kind, `truthy` { field, negated }, that serializes to exactly the two spellings the builder has always written. Spacing-divergent forms (`${ f }`, `not( ${f} )`) stay raw via the self-check. (b) `${f} != ''` / `${f} = ''` parsed cleanly as kind 'answered' and were dropped at the same spot. They now map to the ref / not clause and carry the author's spelling as Clause.source, which clauseToRule re-emits while it still describes the clause. An unedited reopen writes the same bytes; `${f} != ''` is never rewritten to `${f}`. Pinned separately: a bare `${f}` fixture exercises only the truthy path, `${f} != ''` only the answered path. All four reducer tests and the three positive parser round-trip tests fail on bff69cb and pass here. Three UI consumers that switch exhaustively on Rule['kind'] gained a branch for the new kind (modal row, decisions prose, calc prose). Verified: shared tests 792 pass / 0 fail; typecheck clean; corpus sweep output byte-identical to master; a cell-level serialize(parse(x)) check over 4034 relevant/constraint/choice_filter cells in seven configs shows the same 37 pre-existing drifts as master and none new; 21 relevant cells now open as clauses that were raw before. Co-Authored-By: Claude Fable 5.1 --- client/src/ui/CalculationBuilder.tsx | 3 + client/src/ui/DecisionsView.tsx | 2 + client/src/ui/RelevantRuleBuilder.tsx | 22 ++++ .../conditionBuilder/conditionReducer.test.ts | 105 ++++++++++++++++++ .../src/conditionBuilder/conditionReducer.ts | 52 ++++++++- .../relevantParser.truthy.roundtrip.test.ts | 66 +++++++++++ shared/src/xlsform/relevantParser.ts | 31 ++++++ 7 files changed, 278 insertions(+), 3 deletions(-) create mode 100644 shared/src/xlsform/relevantParser.truthy.roundtrip.test.ts diff --git a/client/src/ui/CalculationBuilder.tsx b/client/src/ui/CalculationBuilder.tsx index 7958055..3a8e6df 100644 --- a/client/src/ui/CalculationBuilder.tsx +++ b/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`; } diff --git a/client/src/ui/DecisionsView.tsx b/client/src/ui/DecisionsView.tsx index c0396f7..e558b27 100644 --- a/client/src/ui/DecisionsView.tsx +++ b/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': diff --git a/client/src/ui/RelevantRuleBuilder.tsx b/client/src/ui/RelevantRuleBuilder.tsx index dc42aa4..27c9b75 100644 --- a/client/src/ui/RelevantRuleBuilder.tsx +++ b/client/src/ui/RelevantRuleBuilder.tsx @@ -483,6 +483,28 @@ function RuleRow(props: { ); } + 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 ( +
+ props.onChange({ ...rule, field: v })} + /> + + +
+ ); + } 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 diff --git a/shared/src/conditionBuilder/conditionReducer.test.ts b/shared/src/conditionBuilder/conditionReducer.test.ts index 2fa719c..75c9d12 100644 --- a/shared/src/conditionBuilder/conditionReducer.test.ts +++ b/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}'); +}); diff --git a/shared/src/conditionBuilder/conditionReducer.ts b/shared/src/conditionBuilder/conditionReducer.ts index debed51..4f909c3 100644 --- a/shared/src/conditionBuilder/conditionReducer.ts +++ b/shared/src/conditionBuilder/conditionReducer.ts @@ -31,8 +31,10 @@ * legacy fragment-append path was removed in commit B (§3.7). */ import { + parseRelevant, parseRelevantGrouped, serializeAnyParsed, + serializeRelevant, type AnyParsed, type GroupedExpression, type ParsedExpression, @@ -62,6 +64,17 @@ export interface Clause { op: ClauseOp; /** Empty for ops in {'ref','today','not','selected'-without-value}. */ value: string; + /** + * The clause EXACTLY as the author wrote it, when this clause was + * hydrated from a cell whose spelling differs from what `clauseToRule` + * would emit for the same projection. Today that is `${f} != ''` / + * `${f} = ''` (parser kind `answered`), which the strip shows as + * "has an answer" / "is not selected" — the same labels as `${f}` / + * `not(${f})` — but must never be rewritten into them (T9 invariant 1: + * the preset is a projection; the stored text wins on save unless the + * projection changed). Absent on every clause the user builds. + */ + source?: string; } export type Connector = 'and' | 'or'; @@ -485,9 +498,28 @@ export function isDraftComplete(c: Clause): boolean { } function clauseToRule(c: Clause): Rule { + // A hydrated clause carries the author's own spelling. Re-emit it while + // it still projects to the same clause (field / op / value); the user + // cannot edit a committed clause in place, but a stale `source` on a + // clause that no longer matches would otherwise write the OLD text. + if (c.source !== undefined) { + const parsed = parseRelevant(c.source); + const sole = parsed.rules.length === 1 ? parsed.rules[0] : undefined; + if (sole && !parsed.isRawFallback) { + const projected = ruleToClause(sole); + if ( + projected && + projected.field === c.field && + projected.op === c.op && + projected.value === c.value + ) { + return sole; + } + } + } if (c.op === 'today') return { kind: 'raw', text: 'today()' }; - if (c.op === 'ref') return { kind: 'raw', text: `\${${c.field}}` }; - if (c.op === 'not') return { kind: 'raw', text: `not(\${${c.field}})` }; + if (c.op === 'ref') return { kind: 'truthy', field: c.field, negated: false }; + if (c.op === 'not') return { kind: 'truthy', field: c.field, negated: true }; if (c.op === 'selected') { return { kind: 'selected', field: c.field, value: c.value, negated: false }; } @@ -515,7 +547,21 @@ function ruleToClause(r: Rule): Clause | null { value: r.value, }; } - // answered / date_offset / age / raw don't have a 1:1 in the Clause shape. + if (r.kind === 'truthy') { + return { field: r.field, op: r.negated ? 'not' : 'ref', value: '' }; + } + if (r.kind === 'answered') { + // `${f} != ''` reads as "has an answer" and `${f} = ''` as "is not + // selected" — the same projection as `truthy` — but the spelling is + // kept on the clause so an unedited reopen writes back the same bytes. + return { + field: r.field, + op: r.negated ? 'not' : 'ref', + value: '', + source: serializeRelevant({ combinator: 'and', rules: [r], isRawFallback: false }), + }; + } + // date_offset / age / contact-* / raw don't have a 1:1 in the Clause shape. return null; } diff --git a/shared/src/xlsform/relevantParser.truthy.roundtrip.test.ts b/shared/src/xlsform/relevantParser.truthy.roundtrip.test.ts new file mode 100644 index 0000000..47833fd --- /dev/null +++ b/shared/src/xlsform/relevantParser.truthy.roundtrip.test.ts @@ -0,0 +1,66 @@ +/** + * T9b (#15) — `${field}` / `not(${field})` as a structured rule. + * + * Every positive case calls the serializer and asserts byte identity; the + * negative cases pin that spacing-divergent spellings stay raw (and still + * byte-identical), and that the new kind does not steal `${f} != ''` from + * `answered`. + */ +import { test } from 'node:test'; +import { strict as assert } from 'node:assert'; +import { + parseRelevant, + parseRelevantGrouped, + serializeAnyParsed, + serializeRelevant, +} from './relevantParser.js'; + +test('truthy: `${f}` parses as truthy and serializes byte-identical', () => { + const src = '${lmp_approx}'; + const parsed = parseRelevant(src); + assert.equal(parsed.isRawFallback, false); + assert.deepEqual(parsed.rules, [{ kind: 'truthy', field: 'lmp_approx', negated: false }]); + assert.equal(serializeRelevant(parsed), src); +}); + +test('truthy: `not(${f})` parses as negated truthy and serializes byte-identical', () => { + const src = 'not(${danger_signs})'; + const parsed = parseRelevant(src); + assert.equal(parsed.isRawFallback, false); + assert.deepEqual(parsed.rules, [{ kind: 'truthy', field: 'danger_signs', negated: true }]); + assert.equal(serializeRelevant(parsed), src); +}); + +test('truthy: chains with other kinds and inside a grouped expression', () => { + const flat = "${a} and not(${b}) and ${c} = 'x'"; + const p = parseRelevant(flat); + assert.equal(p.isRawFallback, false); + assert.deepEqual( + p.rules.map((r) => r.kind), + ['truthy', 'truthy', 'comparison'], + ); + assert.equal(serializeRelevant(p), flat); + + const grouped = "(${a} and not(${b})) or ${c} = 'x'"; + const g = parseRelevantGrouped(grouped); + assert.equal(g.isRawFallback, false); + assert.ok('subgroups' in g); + assert.equal(serializeAnyParsed(g), grouped); +}); + +test('truthy: spacing-divergent spellings stay raw and byte-identical', () => { + for (const src of ['${ f }', 'not( ${f} )', 'not(${f} )', 'not (${f})', 'NOT(${f})']) { + const parsed = parseRelevant(src); + assert.equal(parsed.isRawFallback, true, `expected raw for: ${src}`); + assert.equal(serializeRelevant(parsed), src); + } +}); + +test("truthy: does not steal `${f} != ''` / `${f} = ''` from `answered`", () => { + const answered = parseRelevant("${f} != ''"); + assert.equal(answered.rules[0]?.kind, 'answered'); + assert.equal(serializeRelevant(answered), "${f} != ''"); + const notAnswered = parseRelevant("${f} = ''"); + assert.equal(notAnswered.rules[0]?.kind, 'answered'); + assert.equal(serializeRelevant(notAnswered), "${f} = ''"); +}); diff --git a/shared/src/xlsform/relevantParser.ts b/shared/src/xlsform/relevantParser.ts index 35da434..df20126 100644 --- a/shared/src/xlsform/relevantParser.ts +++ b/shared/src/xlsform/relevantParser.ts @@ -50,6 +50,26 @@ export interface AnsweredRule { negated: boolean; } +/** + * A field reference used directly as the boolean — `${field}` ("has an + * answer" in the inline builder) or `not(${field})` ("is not selected"). + * + * T9b (#15): the inline builder has always WRITTEN these two spellings but + * emitted them as `raw`, so on reopen the parser handed back a raw rule and + * the builder disabled every control. A rule the tool itself produced must + * open in the tool, so they get a real kind. Spacing-divergent spellings + * (`${ f }`, `not( ${f} )`) are demoted to raw by the self-check as usual. + * + * This is NOT the same rule as `answered` (`${f} != ''` / `${f} = ''`): the + * two spellings are never rewritten into each other. + */ +export interface TruthyRule { + kind: 'truthy'; + field: string; + /** True for `not(${field})`. */ + negated: boolean; +} + export interface RawRule { kind: 'raw'; text: string; @@ -156,6 +176,7 @@ export type Rule = | ComparisonRule | SelectedRule | AnsweredRule + | TruthyRule | DateOffsetRule | AgeRule | ContactInputComparisonRule @@ -360,6 +381,13 @@ function parseSinglePart(part: string): Rule { if (sel && sel[1] && sel[2] !== undefined) { return { kind: 'selected', field: sel[1], value: sel[2], negated: false }; } + // ${field} or not(${field}) — the reference itself as the boolean. + // Deliberately tight (no inner whitespace): the serializer emits exactly + // this shape, and the §3.1 self-check would demote anything looser anyway. + const truthy = /^(not\()?\$\{([^}\s]+)\}(\))?$/.exec(t); + if (truthy && truthy[2] && Boolean(truthy[1]) === Boolean(truthy[3])) { + return { kind: 'truthy', field: truthy[2], negated: truthy[1] !== undefined }; + } // ${field} = '' or ${field} != '' const ans = /^\$\{\s*([^}\s]+)\s*\}\s*(=|!=)\s*''$/.exec(t); if (ans && ans[1]) { @@ -468,6 +496,9 @@ function ruleToString(rule: Rule): string { // Answered = `${f} != ''`; not answered = `${f} = ''`. return rule.negated ? `\${${rule.field}} = ''` : `\${${rule.field}} != ''`; } + case 'truthy': { + return rule.negated ? `not(\${${rule.field}})` : `\${${rule.field}}`; + } case 'date_offset': { const op = rule.comparator === 'more_than' ? '>' : '<'; const days = UNIT_DAYS[rule.unit]; From f74b77f1df8885af7a45838c619e8fda4954a71e Mon Sep 17 00:00:00 2001 From: Prajwol Shrestha Date: Thu, 1 Oct 2026 17:41:19 +0545 Subject: [PATCH 2/2] test(e2e): "has an answer" / "is not selected" reopen in the inline builder (#15) Two Playwright specs on a throwaway copy of the mini-config fixture: - write `${lmp_date}` and `not(${danger_signs})` through the strip, save, assert the bytes via the API, reload, and reopen both rows as clauses (no "hand-written" status, undo-last-clause present, insert enabled); - the fixture's existing `${lmp_date} != ''` relevant opens as a clause, re-inserts with zero edits as the same bytes, and survives a save. Co-Authored-By: Claude Fable 5.1 --- client/tests/reopen-truthy-answered.spec.ts | 174 ++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 client/tests/reopen-truthy-answered.spec.ts diff --git a/client/tests/reopen-truthy-answered.spec.ts b/client/tests/reopen-truthy-answered.spec.ts new file mode 100644 index 0000000..e5c24ba --- /dev/null +++ b/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 }; + +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 { + 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 { + 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 { + 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 { + 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 }); + } +});