chore(memory): promote strong-fit tasks-and-targets drafts from memory-pipeline for review

[Hareet]

Promotes 35 strong-fit tasks-and-targets drafts from agent-memory/_pending/ into agent-memory/domains/tasks-and-targets/issues/ for squad content review.

Categories: feature (21), bug (9), improvement (5)
Themes: rules engine (banish nools, stale-state migration), aggregate targets & filters, task filtering/telemetry, reporting periods.

All 35 carry domainFit: strong + a ## Domain Rationale section. 2 weak-fit drafts deferred to Stream C.

• 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.

[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 (8772 reproduces the real 283s->63s benchmark exactly). Blockers:

• issue (blocking): identity keys record the PR number on 15 of 35 drafts (verified); 8838 closes no tracked issue.
• issue (blocking): hallucinated identity on 9232. id/issueNumber/issueUrl = 137, but 137 is medic/care-teams#137 (a cross-repo link in the PR body) mis-stamped as cht-core; cht-core#137 is the unrelated "Restyle data records". PR #9232 actually closes #9231.
• issue (blocking): 3 duplicate clusters (8 files): issue 9231 = [9232, 9282, 9317], 9431 = [9486, 9549], 9552 = [9553, 9555, 9569, 9570].
• issue: related_issues: [] empty on every draft; domainFit: strong on every draft. Forced picks: 10390/10432 (cht-datasource data-layer, tied to tasks only via the target docs), 8932 (touches tasks.component.ts and contacts.component.ts equally), 10786 (telemetry + replications.js/purger.js pipeline).
• nitpick: process-narrative leaks into prose on ~11-18 of 35 drafts (named reviewers, "CI green, all 47 checks pass", "the human reverted an AI change").

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

[sugat009]

Content re-review (post-rework). Identity/dedup clean (incl. the 9232→issue 9231 cross-repo fix). Remaining:

• issue (accuracy): 9553 invents a backport version (inline).
• issue (domain): 10390 (target-intervals in cht-datasource) is library extension → data-access (inline). 10432 was relocated to contacts; same treatment applies.
• nitpick (leakage): 10324, 9650 Domain Rationale scaffolding; strip.

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 the detailed data-access proposal comment on #122.

[Hareet]

Both round-2 items are fixed, and putting this branch through the same verification the configuration and messaging batches got turned up a class none of us had seen — one epic reshaped five of these drafts before it landed. Details below, worst first.

Your two items

9553's invented backport version — fixed, and every number in the replacement checks out:

git -C $CORE log -1 --format='%h %s' c8a7f13
# c8a7f13 fix(#9552): handle rules-engine stale state after upgrade  for 4.13.x (#9555)
git -C $CORE branch -r --contains c8a7f13 | grep -E 'origin/[0-9.]+x$'   # origin/4.13.x only
curl -s https://api.github.com/repos/medic/cht-core/pulls/9555 | jq -r '.base.ref'   # 4.13.x

The draft now also records why there is no 4.1.x backport: that line's tip is 62aadbd (4.1.2, June 2023), which predates the October 2024 fix.

10324 and 9650 Domain Rationale scaffolding — stripped.

The thing worth your attention: one epic, five drafts

10324, 10371, 10390, 10436 and 10507 all merged into the 10140_previous-month-targets feature branch and reached master only in that epic's squash, #10423 (622c625427). The epic reshaped them on the way, so their file paths are accurate for their own PRs and wrong as directions to anyone reading master:

the draft names	master has
tests/e2e/default/analytics/analytics.wdio-spec.js	tests/e2e/default/targets/analytics.wdio-spec.js — the directory was renamed
webapp/src/ts/libs/config.ts (created by 10507, renamed by 10436)	nothing; only .mocharc.js survived, and the subtitle logic moved into rules-engine.service.ts
10390's ten target-interval.* files (of the 21 it touched)	target.* — same feature, renamed wholesale

git -C $CORE ls-tree --name-only -r origin/master shared-libs/cht-datasource/src | grep target
#   .../src/local/target.ts  .../src/remote/target.ts  .../src/target.ts     ← no -interval
git -C $CORE log --all --diff-filter=A -- '*target-interval*' | wc -l        # 0 — never on any branch

The four that stay here are now time-scoped with their landed locations. 10390 is not — its vocabulary needs a rewrite, not a patch, and I would rather flag that than half-fix it. It does now carry the same "paths are as of this PR" banner as the others, so nobody reads those ten names as directions to master in the meantime. See below.

Everything else the pass found

Twelve self-contradictions across nine drafts, the same class you identified on #120: the grounding pass corrected the sections that assert mechanism against code and left summary and Design Choices asserting what it had just disproved. Each verified against cht-core before rewriting — 9232 claimed the filter shows "only for multi-facility users" while its own Solution says there is no facility-count gate; 9705's Design Choices claimed the fix throws on write errors when it changed a read catch-all and the file's one bulkDocs still only console.errors; 10623 said filters were reused when overdue-filter and task-type-filter are new components on master.

Then five more that my own first round of fixes created — correcting one section and leaving a sibling saying the old thing, which is the failure mode this whole exercise is about, committed by the person fixing it. 9232 and 9553 each needed three passes before every section agreed.

Then eighteen more across thirteen drafts in a final round of repeated passes, each checked against cht-core rather than reasoned about. Worth naming, because two were mine and several are classes I had not been testing for:

• 10480's summary said the PR generalised the unread-count flow to carry either count. It does not: setBubbleCounter still only carries reports and messages, and getBubbleCounter spreads that and adds task: taskState.overdue?.length || 0 from a different slice. What was reused is the bubble at the display layer, not the dispatch. My own Code Patterns rewrite had said so plainly and I left the summary contradicting it.
• 10480 again: I had asserted "a task due later today still counts" without checking why. It is true, but only because moment(emission.dueDate, 'YYYY-MM-DD') parses date-only, putting a task due today at midnight — already before moment(). The draft now states the mechanism and cites Show a count of overdue tasks in the nav bar cht-core#3943, which asks for exactly "Overdue" and due-"Today" and "not things that are due tomorrow".
• 10436's Testing called two page objects new and both e2e helpers new. git diff-tree on its own PR says both page objects were modified, targets-helper-functions.js was added and aggregates-helper-functions.js was modified. This one ground-claims caught; earlier passes had simply never sampled that sentence.
• 10507 described libs/config.ts as "a helper this PR introduced — was updated to", both at once, and gave it a present-tense home in Code Patterns while the same draft says it is not on master.
• 10436 referred to libs/config.ts twice more without that scoping, and 10371's Code Patterns implied one component records both open and selection telemetry — analytics-filter.component.ts records only the :open keys; the selection keys live in the sidebar filter component.
• 10623's Code Patterns named reports and messages as the components that stopped duplicating lineage filtering. tasks.component.ts dropped its ExtractLineageService.removeUserFacility call too, which is the whole reason the refactor appears in a tasks PR.
• 9232's summary said multi-facility users "had no way to scope the aggregate targets analytics view by place", while its own Problem section says they were blocked from viewing aggregate targets at all. Allow users to view aggregate targets and filter by facility_id and reporting period cht-core#9231 is explicit — "in 4.9.0, users assigned to multiple facility_ids cannot view aggregate targets" — so the summary was understating a regression as a missing feature. Third time this draft's summary needed correcting.
• 10362 called the comparator order-by-due-date-and-priority in two sections. That is the name of its test file; the export is orderByDueDateAndPriority. The draft had both, and used the filename where the symbol belonged.
• 11142 attributed FreetextFilterComponent to the tasks module. It lives in components/filters/freetext-filter/freetext-filter.component.ts — the Tasks page reuses it, which is the actual point, so the draft now says which file it is.
• 10423's Testing listed eight test files under one "Added". Six really were added; the two e2e WDIO specs (analytics.wdio-spec.js, target-aggregates.wdio-spec.js) already existed and were modified. Both ground-claims passes caught this one, which is what recurrence is supposed to look like.
• 10324's Testing opened "Added an e2e WebdriverIO spec … and Karma unit specs …". Its PR adds no test file at all: every spec listed was modified, and the sidebar-filter spec was renamed. The section now says so outright.
• 9553's summary blamed "a CHT upgrade that changes target configuration". The Root Cause blames the #9486 persisted-shape change, and the code agrees — isStale is (state) => !state || !state.targets || !state.aggregate, a pure shape check that never reads configuration. The trigger is upgrading across #9486, full stop.
• 10362's Domain Rationale said the implementation "centers on the task pipeline (new task-notifications service, tasks reducer, …)" while its own Solution says the reducer is not consumed by the service and was only changed to import a comparator. Same class as everything else on this branch, found on the fourth round of passes.
• 9232 wrote "added new translation keys (api/resources/translations/messages-en.properties)". The keys are new; the file is not.
• 9553's Solution said the migrated emissions are rewrapped "so handleIntervalTurnover can read them against the active interval", while Design Choices said the same function writes the previous interval's target doc. Design Choices is right: the function returns early when stateCalculatedAt falls inside the current interval, and otherwise aggregates against calendarInterval.getInterval(monthStartDate, stateCalculatedAt) — the stale interval — and stores that. Reading against the active interval is the one thing it never does. And true to form, my correction introduced the next finding: it named calendarInterval.getInterval and stateCalculatedAt — both real at the anchor, both gone from master because #9718 (in this batch) removed the whole mechanism. The drift check caught that in three of the next four passes; the sentence is now time-scoped and points at the 9718 draft.
• 8772's summary described the short-circuit as firing "when the key count reaches 500 (the guard bypasses only length < MAX_QUERY_KEYS)". That is correct and almost unreadable — the parenthetical states the inverse condition, and two independent passes read it as contradicting the Solution. The guard is !params?.keys || params.keys.length < MAX_QUERY_KEYS; the summary now says so in one direction only.

I also swept all three domains mechanically for any path a draft lists that is absent from origin/master and then refers to elsewhere without scoping. Fifteen hits: ten are past-tense statements about what the PR did and are correct as written (10279 removing emitter.nools.js is the point of that PR), and five are present-tense claims in 3406, 4039 and 4374 — all three of them anchorless drafts outside #120's diff, already carved out. Nothing in scope on any of the three branches.

All 25 timestamps were stale — the relink, dedup and grounding commits never bumped them.

10390 is left for the data-access work

Your inline comment asks to move it to data-access with secondaryDomains: [tasks-and-targets]. I have left it here — whether it ends up moved or simply carries a secondary domain is exactly the sort of thing that taxonomy change should decide — on the strength of your own recommendation on #122:

Ship data-access + secondaryDomains as one coordinated schema/taxonomy PR (alongside the agreed data-access work, not through these content chores) … Not blocking any single PR.

Doing it here would mean three open PRs racing to add the same enum value, and a corpus where one extender moved while the other seven did not. There is now a second reason: 10390's entire file and API vocabulary needs rewriting to the landed target.* names, so it wants a rewrite by someone thinking about cht-datasource, not a re-key bolted onto a tasks PR. A proposal for that work is being written up separately, and an early count suggests the datasource scatter is considerably wider than the drafts named on these three PRs. If you would rather it move now, say so and I will do it — but I think the sequencing you wrote is the right one.

Gate

validate-schema 64/0. verify-drafts --online (identity, duplicates, near-miss symbols, leakage, cross-references, timestamps): 0 blocking, 0 warnings, 0 unverified across all 25. ground-claims: 0 ungrounded, 0 unverifiable, 0 anchor-unusable, 0 unmatched code fences, ~390 grounded per pass, all 25 anchored — the five epic children via #10423's squash 622c625427, the other twenty at their own commits. check-coherence: 0 contradictions.

Those are not single-pass numbers. On this exact content the gate ran four ground-claims passes and three coherence passes, all clean, after a sequence in which almost every pass had turned up something the one before it missed. That is the point of the count, not decoration.

Two stale-as-written items remain, and they are disclosed rather than fixed — 9232 names analytics-target-aggregates-sidebar-filter.component.html and its spec, true at its anchor and renamed to analytics-sidebar-filter.* when the 10140 epic generalised the sidebar past target-aggregates. They are what stale-as-written exists to record: accurate when written, moved since — and the draft's own epic banner already tells the reader its paths are PR-era.

I also re-ran configuration and messaging under the corrected tooling to be sure none of the five tool fixes weakened an earlier verdict: configuration 99 grounded / 0 ungrounded, messaging 225 grounded / 0 ungrounded (its 27 unverifiable are the five anchorless drafts carved out of #120, unchanged). One of the fixes actually removed a false finding that had appeared on #120's branch — 10230's nepal-doit-sms, where extraction asked whether nepal-doit-sms.js contains its own filename.

Four caveats I would rather state than have you find. The first is the one that matters.

• Both checkers sample rather than exhaust, and I can now put a number on it. Two ground-claims passes over byte-identical drafts each surfaced one ungrounded claim — and it was a different claim each time (11142's FreetextFilterComponent, then 10362's comparator name). Either pass alone would have reported "1 finding, fixed, done" and shipped the other. Coherence behaves the same way: 9232's and 9553's contradictions appeared in every pass, while 9022's surfaced in one of three. So the number below is from repeated passes, and a single clean pass is weak evidence — including mine.
• Five sources of that noise turned out to be tool bugs, and are fixed on the tooling branch. check-coherence was counting pairs whose own rationale read "These do not conflict". ground-claims reported a Root Cause claim as ungrounded when extraction had pinned it to a file the PR created — a pairing no commit can satisfy. It resolved a bare filename onto its real path but not a path tail, so 11142's true "reducers/global.ts only gains a search?: string field" came back as a misattribution. It accepted a multi-word noun phrase ("interval turnover") as a symbol. And it asked whether nepal-doit-sms.js contains its own filename. All five now resolve or discard rather than report, with tests. Each only ever turned a false alarm into silence, so no earlier verdict on any branch is weakened by them — but they do not make either checker exhaustive.
• The coherence gate proves the quotes are real, not that they contradict. That judgement is still a model's, and on this branch two of its findings (10371, 8772) turned out to be correct-but-unreadable prose rather than contradictions. I rewrote the prose anyway — if a checker misreads a sentence twice, a person will too.
• Claim extraction is exhaustive only over code-shaped claims, and one useful class sits outside it. Every repo path and backticked identifier is enumerated deterministically, so that tier is reproducible run to run; semantic claims ("the fix prevents duplicate sends") are still sampled and cannot be enumerated. Specifically: added-versus-modified is checked only when the model happens to sample the sentence — five of the findings above are that class. Making it deterministic means scoping "Added X, Y and Z" across a coordinated list, which I tried and abandoned at 126 false positives out of 164, so I raised the pass count instead of shipping a noisy check. Measured on 13 unchanged drafts, two extraction passes shared only 29% of their claims — which is why the mechanical tier was worth making deterministic, and why I am not claiming the semantic one is complete.

[sugat009]

Round 4. The rework resolved the round-3 self-contradiction class and CI is green, so most of this branch is in good shape: 10279 and 10337 came back fully clean, the 9553 fabricated 4.1.x backport is gone, and leakage is stripped from 10324 and 9650.

Requesting changes on five items I verified myself against the PRs' own diffs and cht-core master. Four are blocking-ish and one is a label error an agent would grep for.

On triage, so this is not a wall of noise: I deliberately dropped about six items that were pure archaeology. Claims like which PR first added a symbol, or what a signature looked like at an old commit, are not worth fixing when the draft states today's reality correctly, and recording the old shape would actively mislead an agent trying to call it. Where a draft states a true-on-master fact with a historical verb ("gains X", "at this commit"), the only real fix is the verb, and I have not filed those. What I have filed is limited to: false on current master, semantically inverted, or fabricated.

One pattern worth naming. The two 9553 paragraphs are byte-identical to the pre-rework version, so 19 commits rewrote everything around them and never touched them. The 10436 and 10362 items are the mirror image of round 3: the pass verified that symbols exist but not that the attribution was right. A sweep of every "this PR added X" and "on master Y" sentence against gh pr view --json files and git grep origin/master would close the whole class.

[Hareet]

Round 5 re-review request:

All five items are fixed. Each was re-derived from the PRs' own diffs and cht-core
master before editing rather than taken on your word, and all five checked out.

Then I did the sweep you asked for in the review body — "every 'this PR added X'
and 'on master Y' sentence against gh pr view --json files and git grep origin/master" — mechanically, over all 25 drafts. It found one more instance of
exactly your class, and the gate found five self-contradictions on drafts you did
not name. Details worst-first.

Your five

10436, the mocha harness (blocking). You were right that the PR removed all of
it; the draft said it added it. From its own file list: config.spec.ts +0/-103,
tsconfig.mocha.json +0/-9, .mocharc.js +0/-2, libs/config.ts +0/-26 — all
deletions. Also fixed the two claims in the same sentence: webapp/tsconfig.spec.json
is the webapp-root karma tsconfig, is on master, and only gained sinon-chai types
here (+2/-1); getValueFromFunction was deleted outright, not folded.

The replacement says where subtitle derivation actually lives, which is your
suggestion on line 64: this PR added getReportingMonth(settings, reportingPeriod)
to rules-engine.service.ts (master :588), surfaced per target as reportingMonth
and rendered via subtitle_translation_key.

9553, the symptom and mechanism (blocking ×2). Both were inverted, and both
were byte-identical to the pre-rework version exactly as you noted. #9552 is a
crash, not silently wrong numbers:

git -C $CORE show 'dc47c51^:shared-libs/rules-engine/src/target-state.js' | grep -n 'Object.keys(state.targets)'
#   43, 67, 227 — a pre-#9486 bare map has no .targets, so Function.keys throws
git -C $CORE show fe795fb --stat | tail -6
#   +1 rules-state-store.js, +13/-6 target-state.js, rest tests — no interval logic
git -C $CORE show 'fe795fb^:shared-libs/rules-engine/src/provider-wireup.js' | sed -n '313,332p'
#   handleIntervalTurnover, isBetween(currentInterval...), getInterval(...) all already there

So the turnover facet is the same shape bug: handleIntervalTurnover read the
persisted blob to write the previous interval's target doc and hit the same throw,
and the base fix's discard-and-rebuild would have dropped those emissions first.
Title, summary, Problem, Root Cause, Solution, Testing and Related Issues now all
tell that story, and the e2e case is named — "should handle old format of the
rules-state-store" — instead of being described as a configuration change. Your
line-99 note is in too: what #9569/#9570 added coverage for is the shape check,
migrateStaleState, and commitTargetDoc receiving the previous interval's
aggregate. (One extra: Code Patterns credited the removal to #9714, which is the
issue; the PR is #9718.)

10362, the comparator (blocking). Confirmed on master — task-utils.js exports
only setTaskState, the comparator is reducers/tasks.ts:15, and the service
imports it from @mm-reducers/tasks at task-notifications.service.ts:11. The
draft now states master's location and keeps the round trip (reducer → task-utils
by this PR → reducer by #10701) as explicit history, with #10701's own reason:
task-utils is for the SMS tasks attached to reports, not rules-engine tasks.

10324, the radio labels (blocking). Confirmed: both filters use
targets.this_month.subtitle / targets.last_month.subtitle = "This month" /
"Last month" (analytics-sidebar-filter.component.ts:42-45), and "Previous month"
is zero-hit in messages-en.properties. Summary and Solution now say "Last month"
and note that ReportingPeriod.PREVIOUS is an enum value, not a label.

10371, the telemetry segment. :reporting-period, matching
collectFilterSelectionTelemetry('reporting-period') and the full key its own
Solution already quoted.

The sweep, and what it caught

Your closing note was the useful part of the review, so I mechanised both halves
and ran them over all 25 drafts.

Path half (git grep origin/master): every path a draft names that is absent
from master must sit on a line that scopes it. 49 hits, all accounted for —
relative fragments that do resolve, banner-scoped epic paths, the deliberately
deferred 10390, and 9232's two disclosed stale-as-written entries. Nothing new.

Attribution half (gh pr view --json files, via the API): every add/remove/modify
claim checked against the PR's real file status, and every "this PR added symbol"
claim checked against the PR's own added lines. To show the screen has teeth rather
than just returning green, I ran it against the pre-fix text and it reproduces
your blocking finding unaided:

10436:43  [webapp/src/ts/libs/config.ts]   prose says ADDED/new, PR says REMOVED
10436:43  [tsconfig.mocha.json]            prose says ADDED/new, PR says REMOVED
10436:43  [config.spec.ts]                 prose says ADDED/new, PR says REMOVED

On the fixed corpus the file-level screen returns 64 candidates and the symbol-level
screen 12, and every one is the "added a symbol to an existing file" shape —
"Added a dbQuery wrapper in pouchdb-provider.js" — or a removal correctly
attributed to a different PR. I adjudicated all 76 by hand.

It did find one real instance of your class, on a draft you had already reviewed:

10324 credited itself with the sidebar's telemetryKey input. It never touches
it. git log-free check, straight from the two PRs' diffs:

# 10324's additions to the component: userFacilities, showFacilityFilter — no telemetryKey
# 10371's additions: @Input() telemetryKey: string = 'target_aggregates';

Fixed, and it now points at #10371.

Worth flagging one thing the sweep surfaced that I deliberately did not
enshrine, because it is the archaeology you said not to file. #10371's diff binds
targetModuleId = "targets" in analytics-targets.component.html while the
@Input it adds in the same PR is telemetryKey — the attribute matched nothing
at that commit and was corrected to telemetryKey later on the feature branch, so
no commit on master ever contains targetModuleId. Recording the dead name would
mislead a reader; the draft states the landed form and stops there.

Five more the gate found on drafts you did not name

9486 took three passes because my first two fixes were each wrong in a new way
— the failure mode this whole exercise is about, committed twice by the person
fixing it. The summary said targets were computed "only ... when visiting specific
pages" while its own Root Cause described two 120s ensure-freshness debounces
(ENSURE_FRESHNESS_SECS = 120 at dc3ef42ab^, used at :133 and :148). Calling
that "a 120s debounce" then contradicted "two separate"; fixing that left Problem
asserting nothing ran on incoming changes while Root Cause said the hook called
updateEmissionsFor per change. At dc3ef42ab^ that function body is
.then(contactIds => rulesStateStore.markDirty(contactIds)) — it invalidates, it
does not recompute. Problem now says precisely that.

9705 — the Problem opened by putting the swallowed error on the write while
the summary, its own next sentence, Root Cause, Solution and Design Choices all put
it on the read that precedes the write. The read is right.

10390 — Domain Rationale said "all changed files are shared-libs/cht-datasource
plus one API controller"; the PR also changes api/src/routing.js, adds integration
tests under top-level tests/integration/, and touches .mocharc.js and
package.json. (Caught twice: once as filed, once after my own first correction
still under-counted.)

8932 — the title called the flashed strings "empty states" while Root Cause
shows both render only when the has-items flag is true (hasTasks,
hasContacts), which makes them end-of-list messages — the opposite. Title and two
tags follow.

10371 — after the segment fix, the Solution read as though everything new was
on the targets page while summary and Domain Rationale said target aggregates. The
input defaults to target_aggregates, so the aggregates page gets selection
telemetry with no wiring and the targets page overrides the key; all three sections
now say that.

10390 still stays put

Unchanged from last round and for the same reasons: the taxonomy change is your own
sequencing recommendation on #122, and 10390 additionally needs its whole
target-interval.* vocabulary rewritten to the landed target.* names — a rewrite
by someone thinking about cht-datasource, not a re-key bolted onto a tasks PR. The
epic banner keeps anyone from reading those ten names as directions to master.

Gate

validate-schema 89/0/3-skipped. verify-drafts --online 0 blocking, 0 warnings,
0 unverified across all 25. ground-claims 0 ungrounded, 0 unverifiable, 0
anchor-unusable, 0 unmatched code fences, all 25 anchored — the five epic children
via #10423's squash, the rest at their own commits. check-coherence 0
contradictions.

The sequence matters more than the totals. Across the round the gate ran 14
ground-claims passes and 15 coherence passes. The first of each turned up 4
ungrounded claims and 3 contradictions, and almost every pass after that found
something the one before it had missed — three separate rounds on 9486, two on
10390, and one on a draft I had already "finished" twice. It went quiet only at the
end, on frozen bytes, and the last five passes were run against the exact committed
content.

Two gate findings were the gate misreading itself, and they are worth naming
because anyone re-running this will hit them.

The first changed the prose anyway. A pass flagged
sidebar_filter:analytics:<telemetryKey>:reporting-period:select as a symbol that
does not exist — correctly, since it is a placeholder form no literal git grep
can match. It only existed because I had written a concrete-looking key with a
placeholder inside it. 10371 now quotes the real template literal, which is
byte-identical at the anchor and on master
(analytics-sidebar-filter.component.ts:125 both places).

The second I left alone: one pass in fourteen flagged target-interval as
non-existent in 10423, from the sentence "the document type was kept as 'target'
(its actual type) rather than 'target-interval'." The symbol's absence is the
thing the sentence asserts, so the probe is confirming the claim while reporting it
as ungrounded — ground-claims has no notion of a counterfactual. I would rather
keep the rejected name on the record, since 10390's file vocabulary is still built
entirely out of it, than reword a true sentence to satisfy a probe.

Two stale-as-written items remain and are disclosed rather than fixed: 9232's
analytics-target-aggregates-sidebar-filter.component.html and its spec, true at
its anchor and renamed when the 10140 epic generalised the sidebar. That is what
stale-as-written is for, and the draft's epic banner already tells the reader its
paths are PR-era. A third drift item, 8932's setTasksList(hydratedTasks), I did
fix rather than disclose — the local is gone from master, so the name was purely
greppable-and-wrong with nothing gained by keeping it.

[sugat009]

Approving. All five items from review 4845979803 are fixed, and I re-verified each against the PRs' own diffs and cht-core master rather than taking the commit message's word for it.

• 9553 now tells the crash story throughout (title, summary, Problem, Root Cause, Related Issues): TypeError: Cannot convert undefined or null to object at Object.keys in aggregateStoredTargetEmissions, with the 4.12 to 4.13.x report. The turnover paragraph correctly states that detection pre-existed (handleIntervalTurnover's isBetween and calendarInterval.getInterval were both present before #9569) and that #9569 added no interval logic, and it adds that #9718 has since removed the mechanism entirely.
• 10436 states the deletions with exact counts and puts webapp/tsconfig.spec.json correctly on master.
• 10362, 10324, 10371 all corrected; 10324 now documents explicitly that "Previous month" is only the ReportingPeriod.PREVIOUS enum value and never a UI string, which is the distinction that made the original wrong.

Two of these went past what I filed, which I want to credit specifically. On 10436 I could only establish that getValueFromFunction was zero-hit on master; you found the actual successor, and it checks out: getReportingMonth is at rules-engine.service.ts:588 on master and PR 10436's own diff adds it. On 10362 you recorded the full round trip (added to the reducer by #9980, moved to task-utils here, reversed by #10701) rather than just correcting master's location.

The self-found fixes in 58a6ca9 and 4ec1d12 are real precision gains rather than churn, and I found no new error introduced by them. The standout is 9486: the old summary said targets were only computed on page visits, and the correction that there were already two once-per-session 120s ensure-freshness debounces verifies exactly at the anchor's parent. ENSURE_FRESHNESS_SECS = 120, one debounce calling fetchTaskDocsForAllContacts() and one calling fetchTargets(), each armed once inside the isEnabled block and never re-armed. That is a claim I never flagged, caught by your own gate.

Also worth noting the method: re-deriving each item from primary sources instead of trusting the review is the right instinct, and it is what caught my own overreach earlier in this batch.

Pre-merge checks: CI green on all four at 58a6ca9, diff confined to agent-memory/, and schema.json is byte-identical to main now that #120 and #130 have merged, so it carries no duplicate change.

One open thread I am deliberately not blocking on. The 10390 domain question (cht-datasource work belonging in data-access) was parked on 2026-07-21 pending the domain existing. #152 now adds data-access and secondaryDomains, so that is answerable whenever you want to sequence it, and this PR's reword of 10390's rationale (naming the controller, the routing.js registration and the test wiring) makes the file set read as even more clearly library-centric. Happy for that to be a follow-up rather than a change here.

Resolving my stale threads on this PR since the underlying items are all addressed.