Repository navigation
fix: "has an answer" / "is not selected" rules reopen in the inline builder (#15) - #23
Open
PrjShrestha wants to merge 2 commits into
Open
PrjShrestha wants to merge 2 commits into
PrjShrestha wants to merge 2 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>
This was referenced Oct 1, 2026
witash
approved these changes
Oct 12, 2026
witash
left a comment
There was a problem hiding this comment.
code looks good
not part of this issue exactly, but I seems to be doing parsing with regex. I wonder if that's the best way to do this or if it would be more maintainable long term to do the parsing differently.
Not for this PR though.
wasn't able to reproduce the issue locally on master or the fix on this branch with the default config because of other issues, I don't think that needs to block this PR
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 #15 (T9b). Parent epic #9. Plan:
docs/plans/9_complex_logic_calculation_relevant_constraint/README.md.What: "has an answer" and "is not selected" rules written by the inline builder reopen in the inline builder as picked clauses; a hand-typed
${f} != ''opens as a clause and saves back unchanged.Why this was necessary: the builder wrote
${f}andnot(${f})correctly but could not read its own output. An author who came back the next day found the rule frozen as hand-written text with every control greyed out, which is the fastest way to lose trust in the tool.What it solves: what you build today is editable tomorrow, and the no-normalise promise holds for the simplest rules too.
Demo video and the "what an author can do" table: #15 (comment)
What
The inline builder could write "has an answer" (
${f}) and "is not selected" (not(${f})) but could not reopen them. Two causes met atruleToClauseinconditionReducer.ts:clauseToRuleemitted both askind: 'raw', so a reopen parsed them as raw and fell torawFallbackwith every control disabled. They now have a real parser kind,truthy { field, negated }, serialized to exactly the two spellings the builder always wrote.${ f }andnot( ${f} )stay raw through the self-check.${f} != ''/${f} = ''parse cleanly asansweredand were dropped at the same line. They now map to the ref / not clause and carry the author's spelling asClause.source, re-emitted while it still describes the clause. An unedited reopen writes the same bytes.${f} != ''is never rewritten to${f}.Three consumers that switch exhaustively on
Rule['kind']gained a branch fortruthy(modal row inRelevantRuleBuilder, prose inDecisionsViewandCalculationBuilder). No other UI change.Tests
Pinned separately, as the ticket asks: a bare
${f}fixture exercises only the truthy path,${f} != ''only the answered path; each test asserts the parser kind so the two cannot be confused. All seven new tests fail onbff69cband pass here. Every round-trip assertion calls the serializer.Validate
pnpm --filter @cht-ui/shared build && testpnpm typecheckparseRelevantchanged shapepnpm lintmasteralready (164 problems in untouched files); the seven files this PR touches add nonenode scripts/smoke-parser.mjs geriatric-workflow/forms/app/pregnancy.xlsxnode scripts/corpus-sweep.mjsmaster(theextraSheets/ contact-summary drift it reports is pre-existing)serializeAnyParsed(parseRelevantGrouped(x)) === xover 4034 relevant / constraint / choice_filter cells in seven configsmaster, none new; 21relevantcells now open as clausesFound on the way, not fixed here
The 37 pre-existing drifts are all-raw chains with a double space around
and(gandaki / lumbini FCHV training constraints, one nssd relevant).parseRelevantskips the self-check when every split part is raw and rejoins with single spaces. The inline builder keeps the original text on its raw path, so the bytes are safe through the UI today, but the self-check is not authoritative on that path. Will be handled in 9a (#14), where the self-check rule lives.