codecov/patch under-reports coverage in templated code: llvm-cov's per-instantiation line records are never aggregated

[Yaraslaut]

Problem

scripts/coverage.sh already has a fix for one class of per-template-
instantiation noise in llvm-cov's coverage export
(scripts/aggregate_lcov_branches.py, added for branch/BRDA records —
see that script's own docstring): llvm-cov export -format=lcov emits a
separate coverage record per template instantiation of the same source
line, so a header-only function instantiated with N different type
arguments produces N copies of every line inside it. For branches, the
existing script aggregates these by source location before generating the
final .lcov, so a branch fully covered in aggregate isn't scored
"partial" just because one particular instantiation didn't take an arm.

The same duplication happens for plain line-hit counts (DA: records),
and nothing aggregates those. aggregate_lcov_branches.py's main loop
passes every DA: line straight through unchanged
(scripts/aggregate_lcov_branches.py's final else: out.append(line)
branch) — it only rewrites BRDA/BRF/BRH. If instantiation A of a
function hits line 42 and instantiation B (a different lambda/model type)
never does, llvm-cov — and therefore Codecov's codecov/patch check —
counts line 42 as uncovered, even though the underlying source line is
exercised by real test coverage.

Confirmed real impact

Investigating PR #88's stuck codecov/patch percentage (94-95% against a
97.37% target) surfaced this exact pattern three times, each independently
confirmed via CI logs proving the "uncovered" line's owning test actually
ran and passed:

• examples/common/testkit/pump.hpp: pumpUntil<Pred>'s timeout branch
  (lines 74-79) and awaitQt's deadline-throw (112-113) each appeared
  15+ times in the coverage HTML report — once per distinct lambda-type
  instantiation across the whole test suite. Both are genuinely tested
  (test_pump.cpp's "pumpUntil returns false on timeout without hanging"
  and "awaitQt timeout does not leave dangling references" cases); every
  other call site's own instantiation just never happens to time out,
  which is correct passing-test behavior, not a gap.
• include/morph/core/remote.hpp:747 (acquireSharedInstance's final
  "ok" reply) — every line in its enclosing locked block shows covered;
  this exact line, immediately after the block, does not, despite an
  existing test ("two connections sharing a key reach one instance")
  demonstrably reaching it per CI logs.
• examples/common/testkit/backend_rig.hpp's client<T>() bounds-check
  throw (lines 297-298/301) — a test named exactly
  "BackendRig::client() throws out_of_range past nClients in Socket mode"
  already exists and passes per CI logs, yet the line shows red; client<T>
  is instantiated with several different Model types across the test
  file (RigProbeModel, RigCounterModel, RigBlobModel), each a
  separate instantiation.

This inflates the real uncovered-line count in any header-only,
heavily-templated file (which describes most of morph's core: bridge.hpp,
completion.hpp, registry.hpp, forms.hpp, the ladder's own testkit
headers) well beyond what real test coverage actually leaves untested,
making codecov/patch's percentage systematically pessimistic for such
files and the target correspondingly harder to hit through test-writing
alone — no additional test can fix a report-level double-count.

Proposed fix

Extend scripts/aggregate_lcov_branches.py (or add a sibling step in
scripts/coverage.sh) to aggregate DA: line-hit records the same way it
already aggregates BRDA: branch records: for each source file, sum hit
counts across every instantiation's copy of the same line number before
emitting the final DA:<line>,<hits> record, instead of passing each
instantiation's own DA: line straight through. The coverage.json export
coverage.sh already generates (llvm-cov export -format=lcov step,
consumed for the BRDA aggregation) has the same per-instantiation
"segments"/hit-count data needed to do this for lines — no new llvm-cov
invocation should be required, only extending the existing script's
line-record handling to match its branch-record handling.

Not in scope here

This issue is about the tooling gap, not about writing more tests. Recent
work (PR #88) found and fixed several genuinely-real coverage gaps in
remote.hpp/backend_rig.hpp/presenter.hpp (deterministic preconditions
and forced concurrency races that had never been exercised) before
concluding the remainder is this reporting artifact — that work should not
be redone once this is fixed; re-running the fixed tooling against the
current test suite should simply show the true, already-adequate coverage
for the lines this issue describes.

[Yaraslaut]

Investigated this while working through the broader coverage-tooling cleanup
(#93/#94, and a few follow-ups). Checked the premise directly against real
data captured from this repo's own CI (a coverage.lcov.raw + coverage.json
pair downloaded from an actual clang-coverage run, same toolchain this repo
uses) rather than assuming it -- and the specific claim doesn't hold up:

llvm-cov export -format=lcov's DA: records for include/morph/core/ bridge.hpp (942 segments in the JSON export, heavily templated) show zero
duplicate line numbers in the raw lcov -- 875 DA: records for 875 unique
lines, no line appearing twice. Checked the three examples this issue names
specifically:

• include/morph/core/remote.hpp:747 -- exactly one DA:747,0 record, not
  several conflicting ones.
• examples/common/testkit/backend_rig.hpp's lines 296/298/301 -- the JSON
  export's segments array shows one entry per (line, column) with real,
  nonzero hit counts (16, 15, 3+19), not per-instantiation duplicates.

This means llvm-cov's own lcov exporter already sums/merges hit counts
across template instantiations for line records (DA:) -- unlike branch
records (BRDA:), which really do duplicate per instantiation (confirmed;
that's what scripts/aggregate_lcov_branches.py already exists to fix). The
two record kinds are not symmetric in this version of llvm-cov, so the
fix this issue proposes -- extend that script to also aggregate DA: the
same way -- would be solving a problem that isn't actually occurring at the
tooling level, at least not in the way described.

Caveat: the data I checked predates several rounds of real test-writing
this session did against remote.hpp/backend_rig.hpp in the meantime, so
I can't rule out that the specific lines named here have simply since
gotten real tests and are correctly green now for that reason, independent
of this issue. But the mechanism itself -- "DA: records duplicate per
instantiation and nothing merges them" -- is not what the raw data shows,
for any of the three files/lines checked.

If this can still be reproduced against a fresh CI run showing the same
"marked uncovered despite a passing test" symptom, please reopen/comment
with the fresh coverage.lcov.raw line for the specific flagged line
number -- but based on what I could verify, I don't think there's a real
DA: aggregation gap to fix here. Recommend closing unless a fresh repro
turns up actual duplicate DA: records for the same line.

[Yaraslaut]

Confirming another independent instance of this pattern, from PR #122 (merged): include/morph/core/model.hpp's new ModelHolder<Model>::onActionLogAttached -- a templated virtual override containing an if constexpr branch -- showed lines 265-267 as uncovered on codecov's patch report (57.14% patch coverage) despite a dedicated test (tests/test_action_log.cpp, "ModelHolder::onActionLogAttached forwards to a model-level attachActionLog...") genuinely exercising exactly that branch and passing in CI.

Verified independently three ways before concluding it's this exact tooling gap, not a real test gap:

1. llvm-cov show's own HTML report (ground truth, aggregate across all instantiations) shows lines 265-269 as covered-line with hit counts 36 and 1 -- genuinely exercised.
2. llvm-cov report's own summary line for model.hpp (94.12% region / 91.07% line, 56 total lines) disagrees with codecov's reported totals for the same file (90.47%, 42 total lines) -- the denominators don't even match, which line-level DA duplication across instantiations would explain (llvm-cov's own aggregate report correctly sums per-instantiation hits when computing its own totals; the raw per-instantiation DA: records feeding the lcov export/codecov path do not, per this issue).
3. The same CI run's log contains warning: 247 functions have mismatched data from llvm-cov itself -- confirmed present, with an identical count, on master's own last coverage run (job 95201233868, commit d423e0e) predating this PR entirely. Worth linking here: this warning may be describing the same underlying per-instantiation data-merge issue this issue's proposed DA: aggregation fix would also address, or a closely related symptom of the same root cause (multiple coverage-mapping regions for one source location not reconciling cleanly). Might be worth checking whether fixing this issue's proposed DA: aggregation also silences (or explains) that warning.

No action taken on PR #122 beyond confirming the underlying code was genuinely covered (per llvm-cov show) and noting include/morph/ is only informational in codecov.yml today, so this didn't block merge -- filing this comment purely as additional confirming evidence for whoever picks up the proposed fix here.