chore(memory): promote strong-fit forms-and-reports drafts from memory-pipeline for review

[Hareet]

Promotes 47 strong-fit forms-and-reports drafts from agent-memory/_pending/ into agent-memory/domains/forms-and-reports/issues/ for squad content review.

Categories: feature (21), bug (20), improvement (6)
Themes: Enketo widgets & XPath extensions, cht-datasource report create/update, SMS parser, form validation/submission hardening.

All 47 carry domainFit: strong + a ## Domain Rationale section. 5 weak-fit drafts deferred for later.

• Base: seeding-claude-cli-v2 (feat(#108): seeding pipeline - CLI provider, domain-rationale, infrastructure domain, concurrency #119) — retarget to main after the schema lands.
• validate-schema: passing, 0 failures.
• Review for content (domain fit + distillation accuracy), not code.

[Hareet]

Needs to be rebased with main after #119 is merged

[sugat009]

Heads up, this one currently has merge conflicts with its base and will need a rebase before it can merge.

[sugat009]

Content review. Prose fidelity is high across a wide sample (10022, 10246, 10730, 8746 all verified faithful). Blockers:

• issue (blocking): identity keys record the PR number on 25 of 47 drafts (verified); 8740 closes no tracked issue.
• issue (blocking): 6 duplicate clusters (13 files): issue 8745 = [8746, 8748, 8752], 9429 = [9434, 9436], 9604 = [9608, 9610], 9835 = [10022, 10246], 10040 = [10071, 10099], 10041 = [10180, 10200]. Plus near-duplicate content: 10922 (#10904) and 11116 (#10700) distill the same attachment-routing feature.
• issue: related_issues: [] empty on every draft; domainFit: strong on every draft. Forced picks: 9512 (route guard across seven *.routes.ts, no form-engine code), 9513 (once-a-day display gating), 11023 (geolocation.service.ts only), 9641 (API startup resilience). Borderline: 9592 (training-materials page that does include training forms).
• nitpick: classifier/seed reasoning leaks into ## Domain Rationale of the 8745 cluster ("seed-3 principle").
• nitpick: category vs slug/PR type: 9414 is bug but slug feat9413; 9840 is feature but the PR is a fix (its key 9844 is correct).

Clean bill: no secrets, no PII, schema 100% valid.

[sugat009]

issue: 8746/8748/8752 are three PRs (base + two backports) for one issue #8745, distilled as three separate memories; they should collapse to one. The "## Domain Rationale" here also leaks the internal "seed-3 principle" phrasing.

[sugat009]

issue (domain-fit): this is an Angular canDeactivate route guard wired across seven *.routes.ts (about/analytics/contacts/messages/reports/tasks); it is navigation plumbing, not forms-and-reports.

[sugat009]

issue: near-duplicate of 10922 (both route Enketo attachments to the owning sub-doc, same fixtures); 10922 closes child #10904, 11116 targets epic #10700.

[sugat009]

Proposal: a first-class data-access domain + a weighted secondaryDomains[] field

This is a cross-cutting taxonomy/schema item spanning #122, #123, #132 (and the merged #129). Not blocking any single PR — it is the decision the team already partly made in #86.

The problem. cht-datasource work has no clean home in the 9 domains, so library-extension drafts are scattered: #122 (10064/10071/10180, plus the dropped 10022/10246), #123 (10390; 10432 relocated to contacts), #132 (10043 PersonQualifier, 10057 PlaceQualifier, 9266/9281 getPeople). This is the context-selection gap raised in #86 (the distiller injects the wrong patterns for these tickets).

Extend vs use (classification rule): a draft whose primary work extends the cht-datasource library API — its anchor PR touches only shared-libs/cht-datasource — gets data-access as primary domain; a draft that merely consumes cht-datasource keeps its product domain with subDomain: cht-datasource. E.g. 10071 (createReport) is 100% cht-datasource → extend; 9755 touches no cht-datasource files → use.

Already agreed. Hareet proposed a first-class data-access domain (organized on cht-core #11174's Contact.v1/Report.v1 / Person/Place hierarchy), and we settled it in the team meeting to add it, to be populated by re-running the #119 pipeline over the datasource tickets, sequenced after #127 Langfuse for before/after traces. No tracker for it surfaces yet — worth filing.

The secondary-domain piece. Some datasource work is about a product area (e.g. 10071 = data-access primary, forms-and-reports secondary). The schema cannot express that today (subDomain is a free-text sub-area; the code-encoded rule is "one primary domain + related_workflows"). Proposed: add a secondaryDomains: CHTDomain[] field — reuses the CHTDomain enum, array-valued, and carries "smaller power" in retrieval (the consumer, calculateSimilarityScore / research + code-gen, weights a primary-domain match fully and a secondary-domain match at a fraction). This recovers the cross-domain breadth singular domain loses (my #135 comment) without two co-equal primaries. Keep it distinct from related_workflows (a CHTWorkflow) and related_domains (the domain graph). Do not auto-emit it from the distiller initially — seed it human-set on the datasource cluster until primary-domain accuracy is solid.

Recommendation. Ship data-access + secondaryDomains as one coordinated schema/taxonomy PR (alongside the agreed data-access work, not through these content chores), then re-key the extenders (domain: data-access + secondaryDomains: [<product>]) and keep consumers in their product domain with subDomain: cht-datasource. NB the 10022/10246 report-datasource content already lives in #132's 9835 contacts draft — more evidence it wants a data-access home.

[sugat009]

Content re-review (post-rework). Identity/dedup are clean. Remaining:

• issue (accuracy): 9227 names a non-existent XPath function (inline).
• issue (domain): the createReport / report-update drafts (10064/10071/10180) extend the cht-datasource library and belong in the agreed data-access domain (inline on 10071).
• nitpick (domainFit): 9592 is strong but is the same training/navigation class as its weak siblings 9512/9513 (inline).
• nitpick (leakage): 11165 Domain Rationale scaffolding; strip.
• (10922/11116: only a minor gap — the contacts-side child issue 10903 has no draft; listing 11116 in source_prs is defensible.)

Cross-cutting (whole promote batch, not blocking this PR alone): filenames still encode the PR number while frontmatter is issue-keyed, and two draft schemas coexist (machine-distilled vs hand-authored) — worth a cleanup pass. Full detail + re-runnable Verify commands are in my consolidated review notes.

The largest cross-cutting item, moving the cht-datasource library-extension drafts to a first-class data-access domain + a weighted secondaryDomains[] field, is in a separate detailed comment on this PR.

[sugat009]

issue (accuracy): this draft (Solution section, ~L30) names the XPath function cht:luhn-check, but the real function is cht:validate-luhn (registered in PR #9220 as 'cht:validate-luhn': luhn; current webapp/src/js/enketo/medic-xpath-extensions.js and the CHT docs use it, added 4.10.0). cht:luhn-check never existed — an agent grepping the corpus would find a non-existent function. Please rename throughout.

[sugat009]

issue (domain): this PR (and 10064/10180) touches only shared-libs/cht-datasource — createReport/report-update library API. Per the team-agreed data-access domain (#86), these library-extension drafts belong in data-access with secondaryDomains: [forms-and-reports], not primary forms-and-reports. See the detailed data-access proposal comment on this PR.

[sugat009]

nitpick (domainFit): graded strong, but this is a training-materials/navigation page — the same class as 9512/9513, which are weak. Re-grade to weak for consistency.

[Hareet]

Request for re-review

All seven are fixed in 57e7c3a, each re-derived before editing. Thank you for
the suggested rewordings — several are used close to verbatim, because they were
more precise than what I would have written.

The part worth your attention is not the seven. It is that three of them were
introduced by this remediation sequence, two by me, and that your protocol has
a step mine did not.

The three I caused

10133 — mine, from 04c3958. I wrote "only one attachment is read" and
"model.xml and form.html are not read at all". Both false, and you are right
that time-scoping cannot rescue them: #10248's own diff adds the three named
reads in updateAttachments. What happened is narrow and worth naming — I opened
getFormDocs in forms.js, found its single named read, and generalised to the
whole PR without opening the second file the same PR changed. Reframed to your
"named reads instead of bulk reads"; Code Patterns and Design Choices carried the
same wrong claim and are corrected with it.

10071 — mine, from a9f5dcf. Fixing one contradiction I asserted another:
that place-create was "added alongside this work by the same #10099". It was
already standing — #10065 (169a02355, 2025-06-25) and #10089 (98a687a80,
2025-06-26), with #10094 moving the surface to Input, all before #10099 landed
on 2025-07-03. Line 59's "Place had none yet" is true at cab214534, so only the
parenthetical changed, per your suggestion.

9755 — from b82840f, the same commit that correctly removed the
whitespace-fallback claim. The replacement sentence was wrong in a new way. Your
rewrite is used.

The pattern: when a fix replaces a wrong sentence, the new sentence is
unverified. ground-claims then checks that its symbols exist and
check-coherence checks it does not contradict a sibling — and a confidently
wrong attribution passes both. Nine ground passes and three coherence passes said
nothing about 10071.

The other four

8656 — sign and scope both wrong, and the distilled commit's own spec proves
the convention (-60 asserts '+01:00'). Your rewording is used verbatim, and
the nitpick too: narrowed to "every #to-bikram-sambat() conversion case",
since the 11 invalid-input cases assert an empty string and are
timezone-independent.

10922 — all three details corrected. Both PRs are merged (into epic branches
on 2026-05-19 and 2026-05-29); 0df57c664 is an interior commit of
10700-photo-capture, not a head, while cc34e08664 is the head of
10700-photo-capture-in-sub-contacts-and-reports; and neither sha is in the
5.1.2 branch, which carries the #11116 squash instead. The frontmatter summary
and the "unmerged epic branch work" concept follow. Your systemic suggestion is
adopted here — the banner now leads with "merged into epic branches on ;
not on master".

8740 — your rewording is used, including the point that #8740's own
Escape-workaround removals cancelled out inside the epic branch and are absent
from 314e79061a^.

8759 — removed from this PR. It duplicated a landed contacts draft's
identity, and re-keying it to 8759 is not an option for the reason you give. I
have not folded its content into the contacts canonical from this branch, because
that file is landed on main and actively being corrected in #132 — editing it
here would collide. The two pieces worth keeping are below, for whoever finishes
#132:

Testing — Unit tests updated for search-request generation
(shared-libs/search/test/generate-search-requests.js), the search service
(webapp/tests/karma/ts/services/search.service.spec.ts), and the
select2-search service
(webapp/tests/karma/ts/services/select2-search.service.spec.ts). An e2e WDIO
spec (tests/e2e/default/enketo/db-object-widget.wdio-spec.js) with a dedicated
test form (tests/e2e/default/enketo/forms/db-object-widget.xml) and
page-object helpers (tests/page-objects/default/enketo/generic-form.wdio.page.js)
exercises the descendant filtering through the widget.

Design Choices — the appearance name descendant-of-current-contact was
chosen over a parent-oriented alternative because the filter is a subtree test,
not a siblings-of test: it matches every contact below the in-context contact,
not only its direct children.

If the team decides forms-and-reports should own this memory instead, that is the
relocation you describe, and a separate change against the landed file.

Your corpus-wide check already exists — I was running it wrong

You suggested adding a corpus-wide issueNumber uniqueness check to close this
class. It is already there: checkDuplicates in verify-drafts is documented
"computed across the WHOLE corpus, including already-landed drafts". It never
fired because I spent this entire effort running --dir …/domains/forms-and-reports,
so the contacts canonical was never loaded to collide with.

The right invocation loads everything and still reports only our drafts:

npm run verify-drafts -- --dir <agent-memory> --changed-only --base origin/main --online
#   40 drafts checked, 0 blocking, 0 warnings, 0 unverified

Run without --changed-only, over the landed corpus, it reports 8 blocking
findings that predate this PR and that per-domain gating has never surfaced:

• issue 8034 — contacts/8034-… and data-sync/9593-… (the pair you documented)
• issue 10792 — a triple: data-sync/10793, 10798, 10799
• data-sync/10399 — filename says 10182, frontmatter says 10183
• messaging/10802 — names task.status and task.due_date; neither exists in
  cht-core (task.state, task.dueDate)

None are in forms-and-reports. Worth a separate corpus-repair pass.

A probe for the class you keep catching

The attribution class is now mechanised rather than left to review.
introduced-by asks "did PR #N add this symbol", which no existing probe asked —
they all ask whether an identifier is real, and in every one of these defects it
was.

Two simpler versions were built first and both produce false accusations on this
same history, which is why the final one is per-file:

• "the symbol exists at #N's parent" refutes #10065 introducing createPlace —
  a true claim — because the old places controller had an unrelated
  createPlace elsewhere in the tree.
• "the symbol appears on an added line of #N's diff" accepts #10099 introducing
  it — a false claim — because its one such line is an import edit in index.ts.

Locality separates them: for each file where #N adds a line naming the symbol,
did that file already contain it at #N's parent? Verified against this history —
#10099/createPlace ungrounded, #10065 and #10089 grounded. The enumerator
only emits the claim when exactly one PR number governs a create-verb sentence,
because "via #10065 and #10089" is the shape a correct draft uses and guessing
between them would manufacture defects.

Gate

On the frozen, committed bytes of 6e43e88:

Check	Result
validate-schema	94 passed, 0 failed
verify-drafts --online (corpus loaded, focused on this PR)	40 drafts, 0 blocking, 0 warnings, 0 unverified
ground-claims	0 ungrounded on passes 56, 57, 59, 60, 61 (over all 40 at da89a6e), and 62, 63, 64 over 10180, the only draft changed since
check-coherence	0 contradictions on passes 70, 71, 72

Two coherence passes (66, 68) were cut short when the session ended — no result,
not a failure. Pass 58's single finding was the --skip-validate artifact
described below and is now suppressed in the tooling rather than papered over in
the prose.

Pass 69 then found one more, and it is worth stating because it is the same
class as everything else here: scoping Input.v1.UpdateReportInput on 10180,
I had written that the type is "absent at this draft's own source_sha" — a
claim about a commit the draft's own Provenance section says is unreachable in a
clone. Not false so much as unverifiable, and unverifiable by my own account
three paragraphs earlier. 6e43e88 replaces it with what the PR file list can
settle: input.ts is not among the two files #10180 changed. Coherence then came
back clean three passes running, and ground three more over that draft.

Disclosed rather than fixed. ~25 stale-as-written items across the
epic-branch drafts (10922, 10071, 10180), whose paths and symbols are real
on their branches and absent from master; those drafts carry explicit banners and
a Provenance section. Plus one claim in 9974 that reports unverifiable
because its anchor does not resolve in the local clone — that draft is
hand-authored with source_prs and no source_sha, which is the anchor weakness
about a third of this domain shares. unverifiable is not a pass, so it is listed
here rather than counted as clean.

What changed, in order

Eleven commits since the round-3 review's e98e5ae:

57e7c3a	the seven review items, each re-derived first
f061fcf	8826 — name the attribute that greps, not only the XLSForm column
ec3adbf	pin the enketo-core event name to the version cht-core actually pins
7eb5727	9301 wiring named precisely; 10784 punctuation
ae4c5ff	10917 selector needs two classes; 9755 Nouveau scoped to master
5f8b6fc	three "true but unfindable" claims (8806, 10922, 10180)
e593d7c	backfill related_issues from cross-links already in the prose
e8d6116	timestamp consistency across every draft this branch touched
c1929b8	9301 — the web component never gets the user summary
da89a6e	name the file that holds showConfirmExit
6e43e88	10180 — stop asserting a fact about an unreachable commit

Five of those eleven exist only because a fix of mine introduced a new defect —
10133 and 10071 in round 2, 9301 twice, and 10180 above. That is the
number worth carrying out of this round, and it is why the checks below moved
into the deterministic tier.

Tooling, so this class stops costing review rounds

On #145, prompted directly by defects from this branch:

• introduced-by probe — "did PR #N add this symbol", which nothing asked.
  Two simpler designs were built first and both falsely accuse on this very
  history; the surviving one is per-file. #10099/createPlace ungrounded,
  #10065/#10089 grounded.
• Three cross-field checks — related-issues-desync,
  missing-domain-rationale, fit-mismatch. Each replaces a defect a sampled
  pass found late or never: 10922's empty related_issues took until coherence
  pass 28; 10071's missing Domain Rationale was never found by any pass.
• Cross-repo disclaimer — a claim the draft places in cht-conf is
  unverifiable, not ungrounded. --skip-validate had been reported as a
  defect on three separate rounds after the prose was already correct.
• Two false-positive fixes in my own tooling, both caught by the gate:
  an attribution that credited a PR with every symbol in the paragraph
  (#10445 introduced \_id``), and a coherence pair the model withdrew by
  verdict rather than by negation.
• A runbook (docs/memory-review-runbook.md) with the invocations that cost
  time when wrong, the convergence bar, and the four-step claim protocol — plus a
  correction, because its headline advice about --changed-only did not work and
  only running it proved that.

[sugat009]

Round 4, covering only the delta e98e5ae..6e43e88 (11 commits, 21 files). All seven round-3 threads
are resolved, and six of the seven fixes verify exactly against origin/master and the real cht-core
PRs. The 8759 duplicate is cleanly deleted. The heavy rewrites are the strongest text in this PR so
far; the 10922 banner and the 8740 testing section both check out clause by clause.

I am requesting changes on two items only, both introduced by this round's repair commits. Same
pattern we have now seen on three PRs: the fix resolves the old finding and adds a fresh wrong fact.
Both have committable suggestions attached, as do four smaller items, so the whole round can be
resolved from the GitHub UI with one batch commit.

One decided item at the end: related_issues holds cht-core issue numbers, so the backfilled ids
with no promoted entry are fine as they stand. Details in the note on 10509...md:19; no change
requested there.

[sugat009]

issue (blocking): The repair swapped one wrong mechanism for another. app.component.ts performs no
instance[id="..."] lookup; that selector exists in exactly one place in the webapp tree,
form.service.ts:105 (modelHasInstance), a service the web component bypasses. What
app.component.ts actually does is tag the summary it is handed ({ id: 'contact-summary', context: value }, :82) and call EnketoService.renderForm directly (:217). The load-bearing conclusion is
right, the web component never gets the user summary; only the mechanism is wrong. An agent that greps
app.component.ts for the quoted selector finds nothing, which is the exact failure mode these drafts
exist to prevent.

Suggested change

- The standalone `webapp/web-components/cht-form/src/app.component.ts` takes the subject summary as a `contactSummary` input and looks it up as `instance[id="contact-summary"]`, which is the instance-id shape this PR gave it. It does **not** receive the user summary: `user-contact-summary` is webapp-side only, in form.service.ts and xml-forms.service.ts, so an embedded form cannot read it

- The standalone `webapp/web-components/cht-form/src/app.component.ts` takes the subject summary as a `contactSummary` input and tags it with the instance id this PR gave it (`{ id: 'contact-summary', context: value }`), then hands it to `EnketoService.renderForm`, which injects it as that named instance. It does **not** receive the user summary: the `user-contact-summary` instance is created only in the webapp's `form.service.ts` (whose `instance[id="..."]` probe the web component never runs, since it bypasses `FormService`), and `xml-forms.service.ts` exposes the same data to context expressions as `userSummary`, so an embedded form cannot read it

[sugat009]

issue (blocking): This sentence's central claim is false, and it was also the premise for commit
6e43e88, which removed a true, checkable anchor claim from the Solution section. The sha
70b7be0b4 is not "absent from a clone": it is present and reachable from
origin/9835-add-cht-datasource-apis-for-creation-and-update-of-contacts-and-reports, and
git log --all --grep='(#10180)' returns it. So claims can be verified at this draft's own
source_sha, and the per-child split is checkable in git history after all. Contrast with 9755, whose
anchor sha genuinely is unreachable; that one you scoped correctly this round.

Suggested change

PRs #10180 and #10200 were child PRs of the `9835-…` epic branch and are stamped nowhere in cht-core's history — `git log --grep='(#10180)'` finds nothing, and `source_sha` (`70b7be0b4`) is this PR's own merge commit into that epic branch — GitHub still reports it as that PR's merge commit, but it is absent from a clone because the epic squashed it away. The work reaches master only through the epic squash `f382785be` — `feat(#9835): add cht datasource apis for creation and update of contacts and reports (#10083)`. Every path and symbol below is stated as of that squash; the per-child split between #10180 and #10200 comes from the PR descriptions, not from anything verifiable in the git history.

PRs #10180 and #10200 were child PRs of the `9835-…` epic branch, so nothing on `master` is stamped with them — `git log --grep='(#10180)' origin/master` finds nothing. `source_sha` (`70b7be0b4`) is this PR's merge commit into that epic branch and is still reachable from `origin/9835-add-cht-datasource-apis-for-creation-and-update-of-contacts-and-reports`, so claims below can be checked at that commit. The work reaches master only through the epic squash `f382785be` — `feat(#9835): add cht datasource apis for creation and update of contacts and reports (#10083)`. Every path and symbol below is stated as of that squash; the per-child split between #10180 and #10200 is checkable at the child merge commits on the epic branch.

[sugat009]

suggestion (non-blocking): "what was dropped is the field-list requirement, leaving the two
classes above" describes a subtraction, but the edit was a substitution. The first iteration's selector
was .or-appearance-field-list.or-appearance-hidden, already two classes; review swapped
.or-appearance-field-list for .or-group-data. As written, a reader infers a three-class original.
Line 69 in this same draft states it correctly.

Suggested change

Added a new Enketo widget (HiddenGroup, in webapp/src/js/enketo/widgets/hidden-group.js) whose selector is `.or-group-data.or-appearance-hidden` — a data group carrying the `hidden` appearance — and which adds the `disabled` class to every group it matches, leveraging Enketo's existing behavior of skipping disabled groups during navigation. The widget is registered in webapp/src/js/enketo/widgets.js, which pulls it in by module path — `require( './widgets/hidden-group' )` — rather than by the exported `HiddenGroup` name. Review loosened that matcher, but not to "any group with the `hidden` appearance" — what was dropped is the `field-list` requirement, leaving the two classes above.

Added a new Enketo widget (HiddenGroup, in webapp/src/js/enketo/widgets/hidden-group.js) whose selector is `.or-group-data.or-appearance-hidden` — a data group carrying the `hidden` appearance — and which adds the `disabled` class to every group it matches, leveraging Enketo's existing behavior of skipping disabled groups during navigation. The widget is registered in webapp/src/js/enketo/widgets.js, which pulls it in by module path — `require( './widgets/hidden-group' )` — rather than by the exported `HiddenGroup` name. Review loosened that matcher, but not to "any group with the `hidden` appearance": the `field-list` requirement was replaced by `.or-group-data`, so the selector is still two classes, which is why the Karma spec asserts a group carrying only one of them does not match.

[sugat009]

suggestion (non-blocking): The "two notations" sentence points the conversion at the wrong tool and
stops one notation short. generate-xform.js does not render XLSForm into XForm; the
XLSForm-to-XForm step happens upstream (pyxform, via cht-conf). What generate-xform.js does is the
opposite direction: getChtAttributeEntries (:128-131) reads cht:* attributes off the XForm
instance and stamps them onto the rendered HTML as data-cht-*. That third notation,
data-cht-duration, is the one that actually greps in webapp source.

Suggested change

Custom Enketo widget targeting a specific question/input type in countdown-widget.js; separation of animation concerns into webapp/src/js/enketo/lib/timer-animation.js; custom XLSForm attribute passthrough handled in api/src/services/generate-xform.js, with fixture-based round-trip tests under api/tests/mocha/services/xforms/custom-attributes/. Note the two notations: authors write the XLSForm column `instance::cht:duration`, which exists only inside the `.xlsx` workbook, and generation renders it into the XForm as the `cht:duration` attribute — that second form is the one present in the tree.

Custom Enketo widget targeting a specific question/input type in countdown-widget.js; separation of animation concerns into webapp/src/js/enketo/lib/timer-animation.js; custom attribute passthrough handled in api/src/services/generate-xform.js, with fixture-based round-trip tests under api/tests/mocha/services/xforms/custom-attributes/. Note the three notations: authors write the XLSForm column `instance::cht:duration`, which exists only inside the `.xlsx` workbook; the XLSForm-to-XForm conversion (pyxform, via cht-conf) renders it as the `cht:duration` attribute on the form instance; and generate-xform.js stamps that onto the rendered HTML as `data-cht-duration` (`getChtAttributeEntries`, `:128-131`) — the form that greps in webapp source.

[sugat009]

suggestion (non-blocking): The bullet names db.medic.getAttachment() as the pattern's API, but the
three named reads it describes go through formsService.getAttachment(...)
(generate-xform.js:258-260). Grepping generate-xform.js for db.medic.getAttachment finds
nothing; the raw call exists only inside the wrapper at forms.js:31-33.

Suggested change

- Fetch specific attachments by name with `db.medic.getAttachment()` rather than pulling all attachments in a bulk/full-doc read — `updateAttachments` reads exactly three by name (the XForm XML, its name resolved at runtime rather than a fixed `form.xml`, plus `form.html` and `model.xml`) and writes the generated `form.html` / `model.xml` back (PR #10248)

- Fetch specific attachments by name with `formsService.getAttachment()` (a thin wrapper over `db.medic.getAttachment()`) rather than pulling all attachments in a bulk/full-doc read — `updateAttachments` reads exactly three by name (the XForm XML, its name resolved at runtime rather than a fixed `form.xml`, plus `form.html` and `model.xml`) and writes the generated `form.html` / `model.xml` back (PR #10248)

[sugat009]

nitpick (non-blocking): "master :256-261" starts one line early: 256 is the blank line inside
if (getEnketoForm(doc)), and the three-name Promise.all is 257-261. The substantive claim
(byte-identical to what this PR wrote) is exactly right; I diffed the whole function between the #10248
merge and master.

Suggested change

Dropped `attachments` from both reads: `forms.js`'s `_all_docs` call now passes only `include_docs`, and `generate-xform.js`'s `update` now calls plain `db.medic.get(docId)`. The change is named reads instead of bulk reads: rather than pulling every attachment, `updateAttachments` in generate-xform.js now fetches exactly three by name in one `Promise.all` — the XForm XML, whose name is resolved dynamically by the new `formsService.getXFormAttachmentName(doc)` helper (literally `xml`, or any `*.xml` other than `model.xml`), plus `form.html` and `model.xml`, which are the inputs to `addGeneratedAttachments` (master :256-261, byte-identical to what this PR wrote). The same two names are also written back onto the doc (:243, :247). Large media attachments are never loaded during startup or form processing (PR #10248). PR #10248 changed 5 files in the API layer.

Dropped `attachments` from both reads: `forms.js`'s `_all_docs` call now passes only `include_docs`, and `generate-xform.js`'s `update` now calls plain `db.medic.get(docId)`. The change is named reads instead of bulk reads: rather than pulling every attachment, `updateAttachments` in generate-xform.js now fetches exactly three by name in one `Promise.all` — the XForm XML, whose name is resolved dynamically by the new `formsService.getXFormAttachmentName(doc)` helper (literally `xml`, or any `*.xml` other than `model.xml`), plus `form.html` and `model.xml`, which are the inputs to `addGeneratedAttachments` (master :257-261, byte-identical to what this PR wrote). The same two names are also written back onto the doc (:243, :247). Large media attachments are never loaded during startup or form processing (PR #10248). PR #10248 changed 5 files in the API layer.

[sugat009]

nitpick (non-blocking): "the version webapp/package.json pins" is a caret range, "^7.2.5", which
is a floor. What pins 7.2.5 in practice is webapp/patches/enketo-core+7.2.5.patch (and the lockfile
resolution). The event fact itself is exact; I read BeforeSave() in the installed 7.2.5.

Suggested change

In `prepareForSave`, CHT fired `$('form.or').trigger('beforesave')`, which had two defects: (1) the event name `beforesave` was missing the hyphen. enketo-core's event.js defines the event as `before-save`: in 7.2.5, the version webapp/package.json pins, `BeforeSave()` returns `new CustomEvent('before-save', { bubbles: true })`. And (2) jQuery's `.trigger()` only invokes jQuery-bound listeners and never reaches native `addEventListener` listeners. enketo-core's preload.js updates the `end` value inside a native `before-save` DOM listener (`form.model.evaluate('now()', 'string')`). Since enketo-core 7.x (the Enketo Uplift in CHT 4.0.0) dropped jQuery entirely, that native callback silently never executed, leaving `end` frozen at its preloaded value equal to `start`.

In `prepareForSave`, CHT fired `$('form.or').trigger('beforesave')`, which had two defects: (1) the event name `beforesave` was missing the hyphen. enketo-core's event.js defines the event as `before-save`: in 7.2.5 (webapp/package.json requests `^7.2.5`; webapp/patches/enketo-core+7.2.5.patch pins it in practice), `BeforeSave()` returns `new CustomEvent('before-save', { bubbles: true })`. And (2) jQuery's `.trigger()` only invokes jQuery-bound listeners and never reaches native `addEventListener` listeners. enketo-core's preload.js updates the `end` value inside a native `before-save` DOM listener (`form.model.evaluate('now()', 'string')`). Since enketo-core 7.x (the Enketo Uplift in CHT 4.0.0) dropped jQuery entirely, that native callback silently never executed, leaving `end` frozen at its preloaded value equal to `start`.

[sugat009]

note (non-blocking): Decision to relay, no change requested: related_issues holds cht-core
issue numbers, not corpus entry ids. So all ten backfilled ids stand as they are, including the
seven with no promoted entry (8147, 4543, 7462, 7674, 8072, 5468, 9586); an external issue number is
checkable against GitHub forever, and because entry ids are themselves cht-core-<issueNumber>, a
link that dangles today resolves automatically the day that issue's memory is promoted. Current
corpus practice already matches: all 8 existing related_issues values use this exact format. #138
lands the matching schema description, so please do not edit schema.json here; both PRs touch it
and a second edit would conflict.

The rule going forward: a link to a real issue stays, since it invents nothing; only an inaccurate
description of a linked issue gets flagged. The #9601 prose here and on 9974 ("other recent
contact form work") is hedged but accurate, so both links stay as-is.

[sugat009]

praise: The rewritten epic-branch banner is the most claim-dense paragraph in the delta and every
clause holds, including the three the last round rejected. I verified the merge dates, both squash
shas, the patch-id equivalence of the interior commit, and the branch reachability. Same for the 8740
testing rewrite: the two-sided claim (what the squash carries and what cancelled out inside the epic
branch) reproduces exactly, down to the 4-line spec diff. This banner is the template the round-3
review asked for; worth lifting into TEMPLATE.md at some point.

[Hareet]

re-review request

Every one of your six suggestions was re-derived against cht-core before being
accepted — and that discipline paid for itself immediately, because one of the
six carries a stale mechanism. Five are applied byte-exact (pulled from the
review API and round-trip-compared after writing, bdbf090, with your
co-authorship). The sixth (10180) is amended in a389ae0.

On the UI-batch offer: appreciated, and it would have worked for those five. We
applied locally instead for one reason — the gate runs on frozen committed bytes
before anything lands, and this round's own evidence says that ordering earns
its keep: one of the six suggestions needed amendment the UI cannot make, and
the delta gate rejected two of my own rewordings within minutes of writing them
(details below). Same batch semantics, verification first.

The one suggestion that could not be applied verbatim

Your 10180 conclusion is right and mine was wrong: 70b7be0b4 is checkable,
git log --all --grep='(#10180)' returns it in any clone that has the epic PR's
ref, and 6e43e88 removed a true anchor claim on a false premise. But the
mechanism in your suggested text — "still reachable from
origin/9835-add-cht-datasource-apis-for-creation-and-update-of-contacts-and-reports" —
only resolves in a clone holding a stale, unpruned copy of that branch:

git ls-remote origin 'refs/heads/9835*'   # empty — the branch was deleted after the epic merged
git ls-remote origin refs/pull/10083/head # d92dc2945 — durable
git fetch origin refs/pull/10083/head:refs/verify/pr10083
git merge-base --is-ancestor 70b7be0b4 refs/verify/pr10083; echo $?   # 0

Your clone still has the branch ref; a fresh clone never will. The committed
banner cites the pull ref, which GitHub keeps forever. I want to flag the
symmetry rather than hide it: this is precisely the defect class you filed
against my 9301 fix — right conclusion, wrong mechanism, written from a clone
state the reader will not share. It is why we now re-derive suggested text
too, not just our own.

While restoring the Solution's anchor claim I re-derived what the anchor
actually holds, and it is one notch more specific than what 6e43e88 deleted:
at 70b7be0b4 the update validates the older ReportInput type through
validateReportUpdatePayload; the Input.v1.UpdateReportInput name first
enters input.ts with the epic's #10522 refactor (a89955a9f) and is
src/input.ts:36 on master. The committed text says that.

The five applied verbatim — each verified first

Draft	What was checked	Result
9301	instance[id= at origin/master: exactly one hit, form.service.ts:105; app.component.ts tags at :82, calls renderForm at :217, zero FormService imports	verbatim
10917	first iteration hidden-field-list.js selector was .or-appearance-field-list.or-appearance-hidden; 0eff23be6 deletes that file, adds hidden-group.js with .or-group-data.or-appearance-hidden — substitution, not subtraction; Karma spec asserts single-class non-match at :34-35 and :40-41	verbatim
8826	getChtAttributeEntries at generate-xform.js:128-131 reads cht:* off the instance and emits data-cht-* — the conversion direction you describe	verbatim
10133 (both)	formsService.getAttachment ×3 at generate-xform.js:258-260, Promise.all spans :257-261; raw db.medic.getAttachment only inside the wrapper at forms.js:31-33, zero hits in generate-xform.js	verbatim
10784	webapp/package.json:47 is "^7.2.5", webapp/patches/enketo-core+7.2.5.patch exists; enketo-core 7.2.5's event.js:185-186 is new CustomEvent('before-save', { bubbles: true })	verbatim

10509: decision understood and relayed — issue-number links stand, no
schema.json edit here (#138 owns it).

Why round 4 happened, mechanically

Both blockers share one cause, and it is the same cause as round 3's: a
repair sentence is new prose, and new prose is unverified at birth. In each
case I verified the load-bearing conclusion and then narrated a mechanism
around it from memory of code I had just read elsewhere:

• 9301: I grepped app.component.ts for user-contact-summary (the negative I
  was fixing) and then described how the subject summary is consumed using
  form.service.ts's mechanism — without grepping app.component.ts for the
  selector I was attributing to it.
• 10180: I ruled a claim "unverifiable" by trusting the draft's own
  Provenance sentence as ground truth instead of running git cat-file -e — a
  one-line probe. A negative existence claim got the least verification of any
  claim type when it is among the cheapest to test.

Guard-rails landing on the tooling branch

Three, each mechanizing one of this round's failure shapes:

1. sha-unreachable probe — a sentence asserting a commit is
   unreachable/absent now gets cat-file -e + for-each-ref --contains.
   Reachable → ungrounded, naming the ref. Absent locally → unverifiable,
   because a clone missing refs proves nothing (the mistake your suggestion
   and my draft made in mirror image).
2. Backticked-literal-in-file probe — non-identifier literals like
   instance[id="contact-summary"] bound to the sentence's own named file and
   git grep -F'd there. This catches the 9301 class deterministically: run
   over the corpus, it re-finds your 9301 finding unprompted, suggesting
   form.service.ts — and adds one refinement: the selector was at
   app.component.ts:237 at this draft's own anchor, so the sentence is
   anchor-true and master-stale rather than never-true. Your replacement text
   describes master accurately either way and is what the batch applies.
3. --added-lines exhaustive mode — every sentence added since a base ref
   must have all its deterministic claims adjudicated; any ungrounded fails the
   gate. Sampled full-corpus passes remain for regression; edited prose no
   longer relies on sampling to be re-checked. The diff is computed in the
   corpus repo resolved from --dir, fixing the reason the earlier
   --changed-only advice was withdrawn from the runbook.

Its first live run earned the build cost on the spot. It flagged 9512 — "New
global NgRx state (actions/global.ts, …)" reads as file creation; #9512's diff
says M for all three, only the guard provider is A. Then it rejected my
first two rewordings: "modified rather than created by this PR" still puts
a create-verb near the paths, and "New global NgRx state, carried in the
existing …" lets the adjective reach the file list. The wording that passed
(a744271) puts a clause break between "New" and the paths — which is also how
a human should have to read it. Defect → fix → gate catches the fix → fix again,
in minutes instead of a review round: that loop is what all three checks are
for.

All of it is regression-guarded: the historical defective bytes (10180 at
6e43e88, 9301, 10071 at e98e5ae) are captured as permanent test
fixtures, and a standing-findings invariant re-runs the hermetic tier over the
110 landed drafts at the tooling's merge-base and HEAD — byte-identical output
required (158 findings, 6 blocking, both revisions).

Gate on the final bytes (aa398b0)

Check	Result
validate-schema	64 passed, 0 failed
verify-drafts (corpus loaded, focused on this PR)	40 drafts, 0 blocking, 0 warnings, 0 unverified
--added-lines delta gate, e98e5ae..HEAD	25/25 grounded, 0 ungrounded — exhaustive over code-shaped claims in the whole delta
ground-claims (LLM, 7 changed drafts)	0 ungrounded, 0 stale ×3 (113/128/116 grounded)
check-coherence (LLM, full 40)	0 contradictions ×3 consecutive on frozen bytes

Two things the LLM tiers surfaced on the way to that table, disclosed in full:

The first grounding round (on bdbf090) reported two stale-as-written items
all three passes agreed on — 10784's prepareForSave hook (removed by the
#10700 save-workflow rewrite, cccce201e) and 9512's app.module.ts
(deleted by #9784's Angular 19 standalone migration, a1730c4b1). Both were
true at their anchors and are now time-scoped (aa398b0), and the follow-up
grounding round came back with zero.

The first coherence pass on aa398b0 flagged 10180 and 8225; the next
three passes on identical bytes found nothing, and my logging truncated the
pass-1 details before the report file was overwritten, so I owe you the honest
version rather than a clean-looking table: I re-read both drafts and the only
contradiction-shaped pairs in them are true two-sided mechanisms — 8225's
branch kept functionally enabled while the disabled CSS class hides it
(the patch's actual design, stated in one sentence), and 10180's
"origin/master finds nothing" vs "--all returns it" (different search
scopes, both verified this round). Both match the compatible-pair
false-positive class the coherence checker's withdrawal filter exists for.
The delta gate's drift report also confirms your 9301 fix closed the only
open drift in the changed set; the one remaining note (10071's
Person.v1.createPerson, anchor-true/master-absent) is the disclosed
epic-branch class carried under its banner.

[sugat009]

I reviewed the delta 6e43e88..aa398b0 at aa398b0. All nine round-4 items are done. The five
verbatim suggestions are byte-identical. The 10180 amendment is correct. I re-derived the
mechanism. Your correction of my suggested text was right.

I checked every added sentence against cht-core. One clause in 10180:64 is false. It is inline
with a suggestion block. One nitpick on 10784, also with a suggestion block.

On 9592: keep strong. 9512 and 9513 do not touch form code, so weak is correct for them.

Gates: validate-schema 94/0/3 on the head worktree. Your table says 64, from a different tree.
Merge onto current main is clean. 8759 is removed. 9227 on this branch fixes the
cht:luhn-check error that main still has.

[sugat009]

issue (blocking): The clause "that is the older ReportInput type from src/input.ts" is
false at 70b7be0b4. At that commit, update takes reportInput: Record<string, unknown>
(local/report.ts:137). It narrows the value with isDoc (:138). validateReportUpdatePayload
also takes Record<string, unknown> (:117-119). The file imports ReportInput (:24) and uses
it only on the create path (:95-107). Your main conclusion is correct. Only the type name is
wrong. The same sentence is in the re-review request.

Suggested change

Added `Local.Report.v1.update` to shared-libs/cht-datasource/src/local/report.ts, which persists changes to a report document via the local data context: it validates the incoming update payload (at this PR's own merge commit `70b7be0b4` that is the older `ReportInput` type from `src/input.ts`, checked by `validateReportUpdatePayload`; the `Input.v1.UpdateReportInput` name first enters `input.ts` with the epic branch's #10522 refactor `a89955a9f` and is `src/input.ts:36` on master; `input.ts` is not among the two files #10180 itself changed), loads the original report and its contact by id, asserts the read-only fields (`_rev`, `reported_date`) are unchanged and the form is still supported, then writes through `updateDoc` from src/local/libs/doc.ts. Unit tests were added alongside. Note the naming — the export is `update` inside the `Report.v1` namespace (`Local.Report.v1.update`, `Remote.Report.v1.update`), matching `Person.v1.update` and `Place.v1.update`; there is no flat `updateReport` symbol.

Added `Local.Report.v1.update` to shared-libs/cht-datasource/src/local/report.ts, which persists changes to a report document via the local data context: it validates the incoming update payload (at this PR's own merge commit `70b7be0b4` the payload is an untyped `Record<string, unknown>`, narrowed by `isDoc` and checked by `validateReportUpdatePayload`; `ReportInput` from `src/input.ts` is used there only on the create path; the `Input.v1.UpdateReportInput` name first enters `input.ts` with the epic branch's #10522 refactor `a89955a9f` and is `src/input.ts:36` on master; `input.ts` is not among the two files #10180 itself changed), loads the original report and its contact by id, asserts the read-only fields (`_rev`, `reported_date`) are unchanged and the form is still supported, then writes through `updateDoc` from src/local/libs/doc.ts. Unit tests were added alongside. Note the naming — the export is `update` inside the `Report.v1` namespace (`Local.Report.v1.update`, `Remote.Report.v1.update`), matching `Person.v1.update` and `Place.v1.update`; there is no flat `updateReport` symbol.

[sugat009]

nitpick (non-blocking): The patch file does not pin the version. A patch-package file only
records the version it was made for. webapp/package-lock.json pins enketo-core to 7.2.5 on
master.

Suggested change

In `prepareForSave`, CHT fired `$('form.or').trigger('beforesave')`, which had two defects: (1) the event name `beforesave` was missing the hyphen. enketo-core's event.js defines the event as `before-save`: in 7.2.5 (webapp/package.json requests `^7.2.5`; webapp/patches/enketo-core+7.2.5.patch pins it in practice), `BeforeSave()` returns `new CustomEvent('before-save', { bubbles: true })`. And (2) jQuery's `.trigger()` only invokes jQuery-bound listeners and never reaches native `addEventListener` listeners. enketo-core's preload.js updates the `end` value inside a native `before-save` DOM listener (`form.model.evaluate('now()', 'string')`). Since enketo-core 7.x (the Enketo Uplift in CHT 4.0.0) dropped jQuery entirely, that native callback silently never executed, leaving `end` frozen at its preloaded value equal to `start`.

In `prepareForSave`, CHT fired `$('form.or').trigger('beforesave')`, which had two defects: (1) the event name `beforesave` was missing the hyphen. enketo-core's event.js defines the event as `before-save`: in 7.2.5 (webapp/package.json requests `^7.2.5`, webapp/package-lock.json resolves it to 7.2.5, and webapp/patches/enketo-core+7.2.5.patch is written against that version), `BeforeSave()` returns `new CustomEvent('before-save', { bubbles: true })`. And (2) jQuery's `.trigger()` only invokes jQuery-bound listeners and never reaches native `addEventListener` listeners. enketo-core's preload.js updates the `end` value inside a native `before-save` DOM listener (`form.model.evaluate('now()', 'string')`). Since enketo-core 7.x (the Enketo Uplift in CHT 4.0.0) dropped jQuery entirely, that native callback silently never executed, leaving `end` frozen at its preloaded value equal to `start`.

[Hareet]

Both applied byte-exact (f73b05b), each verified at your cited lines first: Record<string, unknown> + isDoc at local/report.ts:137-138 with ReportInput create-only (:95, :107), and package-lock resolving 7.2.5. The ReportInput clause was mine from a389ae0 — same class as every blocker since round 2: a repair sentence written from adjacent code instead of a fresh read. On the schema count: our 64/3-skipped is this branch's worktree; your 94 presumably counts a fuller tree — same 0 failed either way. Thanks for confirming the 10180 mechanism correction and the 9592/9512/9513 fit decisions — no changes made there per your note.

Hoping for a quick re-review! thanks

[sugat009]

I reviewed the delta aa398b0..f73b05b at f73b05b. Both suggestions from round 5 are applied
byte for byte. I re-checked the two facts at their sources: Record<string, unknown> plus isDoc
at local/report.ts:137-138, and package-lock.json resolving enketo-core to 7.2.5.

validate-schema on the head worktree: 94 passed, 0 failed, 3 skipped. Merge onto current main
is clean. CI is green.

Two nitpicks inline, both one click. The lastUpdated stamps on the two edited drafts say
2026-08-19, but the edit commit is dated 2026-08-25. Same class as your #132 item 10. Approving
now. Commit the two stamps before merge if you agree.