Repository navigation
test(e2e): one journey drives all four builders; a picked relevance and constraint match at runtime on a live CHT (#20) - #28
Open
PrjShrestha wants to merge 7 commits into
Open
PrjShrestha wants to merge 7 commits into
PrjShrestha wants to merge 7 commits into
Conversation
…lected" 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 <noreply@anthropic.com>
…uilder (#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 <noreply@anthropic.com>
… function forms on `.` (#14) Real validation rules are written against the answer itself and real relevants use relative paths; the shared parser only knew `${field}` subjects, so 774 of the 777 constraints in seven real configs opened as plain text. This slice is shared-only and additive. New in shared/src/xlsform/operand.ts: a closed operand grammar (`.`, `${f}` / `../f` with the spelling kept, literals, whitelisted ODK + CHT calls, `+ - * div mod`, parens). On top of it, four additive rule kinds: - expr-comparison `. >= 0`, `string-length(.) <= 100`, `int(format-date(today(),'%Y')) + 57 >= int(.)`, `. > max(coalesce(${a}, 0), ...)`, `. <= today() - 30` - predicate `regex(., '...')`, `selected(., 'none')`, `not(...)` of those - not-group `not(selected(., 'none') and count-selected(.) > 1)` - always-true `true`, `true()`, `1` (text carried, never rewritten) Each carries the clause verbatim as `source`; the serializer re-emits it while it still parses to the same rule, so `.<=100` and `. <= 100` both open AND save back byte-identical, and only a rule the author changed gets canonical spacing. `../field` on the existing comparison / selected / answered / truthy kinds is a `refSpelling: 'relative'` flag, re-emitted exactly as written in either direction. The reducer attaches `source` to any hydrated clause whose canonical emission would differ, so a `../field` rule opens in the inline strip and saves back unchanged. Also: the self-check now runs on all-raw chains too. Splitting on the combinator rejoined `a and b` with one space (six distinct real FCHV / LMP constraints); such a chain is now one raw rule, byte-identical. Consumers that switch exhaustively on Rule['kind'] show the new kinds as the text the author wrote (modal row, decisions / calc prose); a change in the modal turns the rule into a raw fragment, as before. Measured on the seven analysis configs (777 constraint cells): constraint 682 / 777 open fully structured (3 before; 51 are placeholders) relevant 1839 / 3050 drift 0 / 3919 cells (serialize(parse(x)) === x on every cell) `../` share: 410 relative refs vs 4433 ${} refs; 329 cells use `../` Tests: operand grammar; serializer-exercising round trips from non-canonical fixtures for every form in the ticket; `../` hostile fixtures live for both halves (byte identity and opens-as-clause); the all-raw self-check; an "additive" guard that every pre-T9a fixture still yields its old kind; Playwright: seeded `.` / `../` cells survive open-and-save byte-identical, the modal opens a `.` constraint as rows, a `../` relevant opens in the strip. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…scroll to and highlight the new row (#17) Clicking a type tile in the add-question picker committed immediately: no step for required, hint or validation, and the new row was appended after the hidden __* outputs, off-screen, with no scroll and no highlight (UX review finding 3, P0). QuestionTypePicker: a `configure` step between a question tile and the commit — required, a hint per visible language, and a Validation slot (constraint expression with the existing "✎ build" modal, plus a constraint_message per language; 9e replaces the slot with presets). Enter anywhere commits. "add without details" is the one-click behaviour for this question; "Always skip this step" remembers it (localStorage `cht-ui-builder.oneClickTiles`). Structural tiles, the lineage sentinel, hidden rows and edit-type reopens never get the step. A select's choices step leads into the configure step the same way. PickerCommit gains an optional `details` block; FormEditor writes `required = yes`, `hint::<loc>`, `constraint`, `constraint_message::<loc>` only for the values the author set, so an untouched step adds exactly the row the one-click flow added. Insert position: "+ Question" lands directly after the row the author is on (the last focused card, tracked by a focus-capture on the survey tab), via the new shared `insertIndexAfterRow` (surveyEdits.ts, node-tested). A begin-group row counts as "inside the group" (first child), any other row is followed by its new sibling, so every pair stays balanced. With no current row it falls back to `defaultInsertIndex` (before the trailing plumbing calculates). After commit the new row is scrolled into view, focused and flashed for 2.5 s; a row hidden in Simple mode flips the editor to Full first. The Validation slot's field list is the same dependency-ordered, unique-name, non-plumbing list the row card uses (`pickableFieldsBefore`, now one function), computed for the insert position. Tests: surveyEdits.insertAfter.test.ts (after a top-level row, inside a group, after a begin row, trailing-plumbing fallback, unknown id; each asserting structural balance). Playwright add-question-configure.spec.ts: age (integer) with required, hint, constraint and message lands right after lmp_date, flashed and in view, with those cells on disk and every other row's cells unchanged; "add without details"; a question added while on a row inside a group lands inside, after it, balanced; the remembered preference commits on the tile click. Suite: the build specs were written against one-click tiles, so the Playwright profile seeds the preference ON (storageState in playwright.config.ts, which also covers specs that bypass setup.ts); the new spec clears it. demo-1's row-order expectations now reflect insert-after-current-row. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ssages beside the rule; never normalises (#18) Validation is where a program says what a correct answer is, and the tool could show 3 of 777 real rules. With `.` readable (9a) most real rules are a dozen shapes a non-developer recognises by name. This slice gives them a panel and replaces both constraint builders. shared/src/validation/presets.ts (pure, node-tested): the preset model (between, compare-value, compare-field, compare-today, days-from-today, months-from-today, after-all-of, text-length, allowed-chars, pattern, int-value, bs-year, choice-alone, count-selected, always-true, code), `parseValidation` (one parsed rule → one preset; two adjacent numeric bounds → "between"; an `or` chain, a grouped or unparseable cell → one code item), `emitPreset` (canonical XPath), `presetsFor` (the menu per question kind; today() for dates, now() for date-times), `suggestMessage` and `presetComplete`. THE RECOGNISER NEVER NORMALISES. Every item keeps `source`, the rule text as written; `serializeValidation` re-emits it while it still reads as the same preset and writes the canonical spelling only for an item the author changed. `. <= 100 and . >= 70` is displayed as "Between 70 and 100" and saved as `. <= 100 and . >= 70`; edit the maximum and that item becomes `. >= 70 and . <= 99` while its siblings keep their spelling. relevantParser.ts: ParsedExpression gains an optional `separators` (the text between rules exactly as written — ` and\n`, ` and `), set only when a join is not canonical and honoured by serializeRelevant while it still fits the rule count and spells the combinator. Real configs break ~90 chains across a newline or a double space; they were raw, now they open. Additive: canonical cells and consumer-built expressions carry no separators. client/src/ui/ValidationPanel.tsx: a list of sentence-shaped preset rows with inputs, "+ Add rule" per question kind (plus a plain expression), a code toggle, constraint_message per visible language with a suggested text that stops as soon as the author writes their own, and (row editor) the required checkbox with required_message per language beside it. `true` / `true()` / `1` show "This rule always passes: no validation". Incomplete presets stay on screen without writing a broken expression; only complete items reach the cell. The rule and a suggested message are written in ONE row update (two updates in a tick each started from the same stale row and the second won). Mounted in the row editor in place of the constraint expression field (constraint_message inputs leave the hints block; required_message leaves the raw overrides), and in the add-question configure step's Validation slot (9d) in place of the expression box + modal. The inline strip no longer offers the constraint column. Measured on the seven analysis configs (777 constraint cells): 575 open entirely as presets, 61 as presets plus a plain-text item, 51 are placeholders now labelled, 90 stay plain text (mixed and/or inside not(), curly quotes, decimal-date-time arithmetic). Parser-level: 730 of 777 structured. Drift 0 of 777 through parseValidation → serializeValidation, and 0 of 3919 cells through the parser. A form holding every preset's canonical output (40 constraints) compiles with pyxform 4.5 xls2xform, the step cht-conf runs (Docker is not running on this machine; cht-conf itself is installed). Tests: presets.test.ts (every round trip from a non-canonical fixture through the serializer; edited-item canonicalisation; stale source ignored; every catalogue entry emits and parses back to itself; separators), relevantParser separators tests, Playwright validation-panel.spec.ts (age with Between 0 and 20 in the picker → sheet → reopened preset; reverse-order between, tight `.<=today()` and `true()` displayed as presets and saved byte-identical, then one edited item canonical; text length, date not in the future, select-many choice alone, required message, cells asserted on disk). Two earlier specs updated for the panel replacing the constraint box and modal. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nd constraint match at runtime on a live CHT (#20) The June ticket (#9) had two acceptance criteria nobody had run. four-builders.spec.ts — acceptance 3: in one journey on a scratch copy of the fixture, the inline strip writes a relevant (`selected(${chair_rise}, 'pass')`) and a choice_filter (`selected(${danger_signs}, 'vaginal_bleeding')` on a later row — the picker only offers earlier fields), the Validation panel writes a constraint (`. >= 0 and . <= 20` with its message), and the calculation builder writes `${gravidity} + 1` on a fresh calculate row; every cell is read back through the API after a UI save. live-instance-check.spec.ts — acceptance 4: on a renamed copy of the fixture form, author by picking only (gravidity shows when chair_rise includes "pass"; accepts 0..20 with a message), set the sheet's form_id through the builder's API, deploy that one form with cht-conf 6.5.0 inside the cht-ui-builder image (`--add-host` so the TLS name resolves to the host), then as the CHW on the local CHT 5.2 instance: the question is hidden, shows after "Pass", rejects 25 with the authored message and refuses to submit (no report), accepts 10 and the report is in CouchDB with fields.gravidity = "10". The report and the form docs are removed afterwards; the record id is printed. Skipped when the instance or Docker is not reachable. Run on 2026-10-01 against the poc_demo CHT 5.2.0 instance at https://127-0-0-1.local-ip.medicmobile.org:10445 (NSSD config): report 01a0f7da-9c2b-766d-90aa-3aabfd8dd6be, fields.gravidity=10. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
PrjShrestha
added a commit
that referenced
this pull request
Oct 1, 2026
…th test links; progress log row for 9g Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Oct 1, 2026
There was a problem hiding this comment.
🔵 Needs a closer look
It is a large stacked change touching the core round-trip parser/serializer invariant and production form-editor UI, which needs final human review beyond the minor issues flagged.
4 open findings
What changed in this PR
This PR closes the T9 epic's remaining audit work (#20) by adding the two acceptance tests that had never been run: a single Playwright journey exercising all four logic builders, and a live-instance runtime check. Because it is stacked on #27, the presented diff currently carries the earlier 9b/9a/9d/9e commits (parser rule kinds, validation presets, the ValidationPanel, the add-question configure step) and will collapse to the net-new test files once those merge. The net-new content is the two e2e specs plus the issue/plan status updates.
Changes:
four-builders.spec.ts— one journey writes a relevant, a choice_filter, a constraint (Validation panel) and a calculation, then reads every cell back through the API after a UI save (acceptance 3).live-instance-check.spec.ts— authors a form by picking only, deploys it with cht-conf 6.5.0 in Docker, and asserts show/hide + constraint rejection on a live CHT 5.2.0 instance, skipping when the instance/Docker is unreachable (acceptance 4).- Supporting stacked changes to the shared parser, validation presets, condition reducer, survey-edit helpers and form-editor UI (reviewed for consistency with the round-trip invariant).
| File | Description |
|---|---|
client/tests/four-builders.spec.ts |
New all-four-builders journey; header comment for the choice_filter step is stale. |
client/tests/live-instance-check.spec.ts |
New live runtime check; hardcoded admin/CHW credentials. |
shared/src/validation/presets.ts |
Preset recogniser/emitter; a no-op ternary in suggestMessage. |
client/src/ui/QuestionTypePicker.tsx |
Adds configure step; fieldChoiceOptions prop accepted but never consumed. |
client/src/ui/ValidationPanel.tsx / .css |
New preset-driven validation UI; imports verified. |
shared/src/xlsform/relevantParser.ts, operand.ts |
New rule kinds, ../field alias, separators; well covered by tests. |
shared/src/conditionBuilder/conditionReducer.ts |
Verbatim source preservation + projectRule; consistent with tests. |
shared/src/xlsform/surveyEdits.ts |
insertIndexAfterRow helper with unit coverage. |
| issue/plan status docs | Scope checklist and README status table updated. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
| * Absent → the slot is a plain expression box. | ||
| */ | ||
| fieldOptions?: string[]; | ||
| fieldChoiceOptions?: Record<string, ReportFieldChoice[]>; |
Comment on lines
+36
to
+37
| const ADMIN = { user: 'medic', pass: 'password' }; | ||
| const CHW = { user: 'nssd_chw', pass: 'NssdCare!2026x' }; |
| * emitted cells on disk. | ||
| * | ||
| * relevant the inline strip (condition builder) → `selected(${chair_rise}, 'pass')` | ||
| * choice_filter the same strip on the choice_filter column → `${chair_rise} = 'pass'` |
| export function suggestMessage(p: Preset, fieldLabel: (name: string) => string = (n) => n): string { | ||
| switch (p.kind) { | ||
| case 'compare-value': | ||
| if (p.op === '=') return `Must be ${p.isString ? p.value : p.value}`; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #20 (T9g). Parent epic #9. Plan:
docs/plans/9_complex_logic_calculation_relevant_constraint/README.md(on the T9 docs branch).What: one automated journey drives all four builders (relevant, constraint, choice_filter, calculation) and asserts every emitted cell on disk; a second test deploys the form to a local CHT 5.2 with cht-conf and checks, as the CHW, that the picked rules behave at runtime. The #9 checklist and the README status table are updated with links.
Why this was necessary: the original ticket's acceptance said "a condition authored through the picker matches at runtime". Each builder had its own test, nobody had run them together, and nothing had ever checked a real CHT. A rule that looks right in the sheet and misbehaves on a phone is the failure mode that reaches CHWs.
What it solves: evidence instead of assurance. Gravidity hides until Chair rise is Pass, 25 is rejected with the message, 10 is accepted and saved as a report. Every preset the Validation panel can emit also compiles under cht-conf in the image.
Demo video and the "what an author can do" table: #20 (comment)
Stacked on #27 (9e): the four-builder journey uses the Validation panel, so this branch is cut from the 9e tip. The diff collapses to the last commit once #23 → #24 → #26 → #27 merge.
What
Two Playwright specs for the two June acceptance criteria that had never been run, and the audit of the original scope (the issue checklist and the plan README's status table are updated alongside).
four-builders.spec.ts— acceptance 3. One journey on a scratch copy of the fixture: the inline strip writes a relevant (selected(${chair_rise}, 'pass')) and a choice_filter (selected(${danger_signs}, 'vaginal_bleeding'), on a later row since the picker only offers earlier fields), the Validation panel writes a constraint (. >= 0 and . <= 20plus its message), the calculation builder writes${gravidity} + 1on a fresh calculate row. Every cell is read back through the API after a UI save.live-instance-check.spec.ts— acceptance 4. On a renamed copy of the fixture form, author by picking only (gravidity shows when chair rise includes "Pass"; accepts 0..20 with a message), set the sheet'sform_idthrough the builder's API (cht-conf requires it to equal the file name), deploy that one form with cht-conf 6.5.0 inside thecht-ui-builderimage (--add-hostso the instance's TLS name resolves to the host), then as the CHW on the local CHT 5.2.0 instance:01a0f7da-9c2b-766d-90aa-3aabfd8dd6bewithfields.gravidity = "10"The report and the form docs are removed afterwards. Skipped when the instance (
https://127-0-0-1.local-ip.medicmobile.org:10445) or Docker is not reachable.Audit of the original #9 scope
condition-builder.spec.ts,four-builders.spec.ts;../fieldopens since #24 (parser-dot-subject.spec.ts); searchable picker #25validation-panel.spec.ts(#27),four-builders.spec.tscalculationBuilder.roundtrip.test.ts, geriatric build specs; commits06fd03536e6f784362567c66cfcbrelevantParser.chain.roundtrip.test.ts,conditionReducer.test.ts(enter-group-mode); commits0b332bd3aa66ff2770f2apick-preexisting-values.spec.ts,form-data-passing.spec.ts; commits908ddd9be8279fe4fbab17a0aa2eb96e5b1relevantParser.selfSubject.roundtrip.test.ts),scripts/corpus-sweep.mjs, cell-level check 0 / 3919 drift (#24)four-builders.spec.ts(this PR)live-instance-check.spec.ts(this PR), record above777-rule counts after 9a and after 9e are in the plan README (682/777 structured after 9a; 575 fully + 61 partly as presets after 9e; placeholders 51; 0 drift).
Validate
four-builders.spec.tslive-instance-check.spec.ts(Docker + poc_demo instance up)🤖 Generated with Claude Code