refactor(contracts): make the coverage-sharding scripts maintainable

Address review feedback that the test-sharding / coverage scripts were hard to follow, with no
change to results (parallel coverage in CI and locally, identical merged coverage). Rewrite
mergeLcov.ts into a documented, unit-tested pure core (output byte-identical to before) and add
its unit test; extract named helpers in runParallelCoverage.ts; document why runShardCoverage.ts
must bypass the shell (the npx/HH308 brace-expansion, verified) and why compile.ts gates the
coverage compile-skip on solidity-coverage's __SOLIDITY_COVERAGE_RUNNING flag; add a README
explaining the whole shard pipeline. Verified locally: the mergeLcov unit test, parallel coverage
at 461 files / 96.459% lines / 97.160% functions, and the solhint ratchet unchanged at 181.

Signed-off-by: Miguel_LZPF <miguel.carpena@io.builders>

Author: MiguelLZPF

packages/ats/contracts/scripts/tools/coverage-shard/README.md

@@ -0,0 +1,95 @@
+# Test sharding & parallel coverage
+
+This directory + a few test-side files implement two things on top of the ATS mega-asset test suite:
+
+1. **Parallel local test runs** (`npm run test:parallel:ats`) — split the mega-asset core across mocha workers.
+2. **Sharded coverage** — run `solidity-coverage` in parallel and merge into one report. Both in CI (a 4-way
+   GitHub Actions matrix) and locally (`npm run test:coverage:parallel`, one git worktree per shard).
+
+Net effect: CI coverage ~14m → ~7m, local coverage ~11m → ~5m, with the **same** merged numbers as a serial run.
+
+## The constraint that shapes everything
+
+The IAsset suites export a `…Tests(getCtx)` function and **do not self-register** a `describe`. The only file
+that executes them is `ats.test.ts`. Consequences:
+
+- Coverage **cannot** be sharded by directory glob (`--testfiles 'layer_1/**'` would run none of them).
+- `mocha --parallel` shards by **file**, so the whole core in one file = one worker = no speedup.
+
+So a single piece of infrastructure feeds both goals: a discovery + balancing core, driven two ways.
+
+## The pieces
+
+| File                                                         | Role                                                                                                                                                                                |
+| ------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `test/contracts/integration/suiteDiscovery.ts`               | Discover the IAsset suite files; `suiteWeight` (an `it()` count) and `assignBalancedShards` (greedy longest-processing-time bin-packing) to split them into evenly-weighted shards. |
+| `test/contracts/integration/atsShardRunner.ts`               | `runAtsShard(index, total)` — deploy the mega-asset **once** (`loadFixture`) and run only this shard's balanced slice.                                                              |
+| `test/contracts/integration/ats.test.ts`                     | The entry. Reads `ATS_MEGA_SHARD_INDEX/TOTAL` (default `0/1` = every suite, one deploy = the serial CI run).                                                                        |
+| `test/contracts/integration/shards/ats.shard.{1..8}.test.ts` | Physical entries for **local** `mocha --parallel` (one file → one worker → `runAtsShard(k-1, 8)`).                                                                                  |
+| `scripts/tools/coverage-shard/planShard.ts`                  | A coverage shard's file list: the mega entry (in every shard) + a balanced slice of the standalone suites.                                                                          |
+| `scripts/tools/coverage-shard/runShardCoverage.ts`           | Run `hardhat coverage` for ONE shard (CI calls per runner; the local runner calls per worktree).                                                                                    |
+| `scripts/tools/coverage-shard/mergeLcov.ts`                  | Union the per-shard lcovs into one report (preserving line/function/branch).                                                                                                        |
+| `scripts/tools/coverage-shard/runParallelCoverage.ts`        | Local orchestrator: a git worktree per shard, run them concurrently, merge, tear down.                                                                                              |
+| `tasks/compile.ts`                                           | Skips the redundant second compile `hardhat coverage` would trigger (see below).                                                                                                    |
+| `.github/workflows/100-flow-ats-test.yaml`                   | The CI matrix (`coverage-ats`, 4 shards) + the `merge-coverage` job.                                                                                                                |
+
+## Two parallelism mechanisms — and why both exist
+
+|                                            | Parallelism unit                 | How a shard is selected                                  |
+| ------------------------------------------ | -------------------------------- | -------------------------------------------------------- |
+| **Local tests** (`test:parallel:ats`)      | mocha workers (one per **file**) | the 8 physical `shards/ats.shard.N.test.ts` files        |
+| **Coverage** (CI matrix & local worktrees) | separate runners / worktrees     | one `ats.test.ts` parametrised by `ATS_MEGA_SHARD_*` env |
+
+mocha can only parallelise across _files_, so local parallel tests need physical shard files. Coverage gets its
+isolation from separate runners/worktrees, so it needs no extra files — the shard count is just an env var.
+
+## Why worktrees for local parallel coverage
+
+A coverage run **rewrites generated source in place** — the compile hook regenerates
+`contracts/infrastructure/utils/EvmAccessors.sol` and `scripts/domain/atsRegistry.generated.ts`, and
+solidity-coverage instruments the build. N concurrent shards in one checkout would race and corrupt each other.
+A git worktree per shard gives each an isolated working tree (the same isolation CI gets from separate runners);
+`node_modules` is symlinked, not reinstalled. Shard lcovs have worktree-absolute `SF:` paths, so they are
+normalised to package-relative before merging (otherwise the merge can't union them).
+
+## Why an in-repo lcov merger (not `lcov` / `lcov-result-merger`)
+
+- `lcov-result-merger` (npm) silently **drops function coverage**.
+- The system `lcov` CLI reproduces the numbers only with a large `--rc`/`--ignore-errors` flag soup, emits
+  thousands of "inconsistent" warnings on solidity-coverage's output, and rewrites the file in a newer
+  function-record format (`FNL`/`FNA`) whose Codecov support is unverified.
+
+`mergeLcov.ts` is ~100 lines, exact, dependency-free, and unit-tested
+(`test/scripts/unit/tools/mergeLcov.test.ts`).
+
+## Why the compile-skip in `tasks/compile.ts`
+
+`hardhat coverage` compiles the instrumented sources itself and then runs `TASK_TEST`, which compiles **again**.
+That second pass is a wasted ~60s/shard. The override forces `noCompile` while a coverage run is in progress,
+detected via solidity-coverage's own `__SOLIDITY_COVERAGE_RUNNING` HRE flag (set only during `hardhat coverage`).
+
+## `runShardCoverage` must not go through a shell
+
+`solidity-coverage --testfiles` takes one glob; for >1 file we pass a brace list `{a,b,c}`. We run Hardhat's CLI
+directly under `node` (not `npx`), because a shell brace-expands `{a,b,c}` into separate arguments, which Hardhat
+then rejects with `HH308: Unrecognized positional argument …` (verified).
+
+## Running it
+
+```bash
+npm run test                  # serial, all suites — the CI gate (ats.test.ts at 0/1)
+npm run test:parallel:ats     # local parallel tests across the 8 shard files
+npm run test:coverage         # serial single-run coverage (SHARD_TOTAL=1)
+npm run test:coverage:parallel  # local parallel coverage (4 worktrees) — needs `lcov`? no, uses mergeLcov.ts
+SHARD_INDEX=2 SHARD_TOTAL=4 npm run test:coverage:shard   # reproduce one CI coverage shard locally
+```
+
+## Maintenance notes
+
+- **Changing the CI shard count:** edit the matrix `shard: [...]` list AND `SHARD_TOTAL` in
+  `100-flow-ats-test.yaml` (keep them equal). `planShard`/`mergeLcov` handle any count. 4 is the measured
+  sweet spot — beyond it the per-shard ~2m45s instrument+compile floor dominates and wall-clock stops improving.
+- **The 8 local shard files hardcode `N=8`** (`runAtsShard(k-1, 8)`). Changing local parallel granularity means
+  adding/removing files and keeping the indices in sync; `MIN_EXPECTED_SUITES` in `suiteDiscovery` fails loudly
+  if discovery ever drops suites.
+- Adding an IAsset suite needs no changes here — discovery picks up any `…Tests(getCtx)` file automatically.

packages/ats/contracts/scripts/tools/coverage-shard/mergeLcov.ts

@@ -1,71 +1,77 @@
 // SPDX-License-Identifier: Apache-2.0
 //
-// Merge several LCOV reports (one per coverage shard) into a single LCOV report.
+// Merge per-shard LCOV reports into one, summing coverage across shards.
 //
-// Each shard instruments the SAME contract set but exercises a disjoint slice of the tests, so the
-// union of their hit counts is the full-suite coverage. This sums per-line (DA), per-function
-// (FN/FNDA) and per-branch (BRDA) hits across shards and recomputes the LF/LH/FNF/FNH/BRF/BRH
-// summary lines — preserving ALL three metrics. (The off-the-shelf `lcov-result-merger` drops FN
-// records, silently losing function coverage; `lcov` is not always installed. A small in-repo
-// merger avoids both the lost metric and a runtime-fetched dependency.)
+// Each coverage shard instruments the SAME contracts but runs a DISJOINT slice of the tests, so the
+// full-suite coverage is the union: a line/function/branch counts as "hit" if ANY shard hit it.
+// This merger parses the handful of LCOV record types solidity-coverage emits, sums the hit counts
+// per source file, and re-emits one report. See ./README.md for the whole shard→merge pipeline.
 //
-// Usage: npx tsx scripts/tools/coverage-shard/mergeLcov.ts <output.info> <input...>
-//   where each <input> is an lcov file or a directory searched recursively for `*.info`.
+// Why an in-repo merger and not a standard tool:
+//   - `lcov-result-merger` (npm) silently DROPS function records → function coverage lost.
+//   - `lcov` 2.x reproduces the numbers only with a large --rc/--ignore-errors flag soup, emits
+//     thousands of "inconsistent" warnings on solidity-coverage's output, and rewrites the file in a
+//     newer function format (FNL/FNA) whose Codecov support is unverified.
+//   This stays exact, dependency-free, and unit-tested (test/scripts/unit/tools/mergeLcov.test.ts).
+//
+// LCOV record types, one block per source file (see `man geninfo`):
+//   TN:<test name>                          SF:<source file path>
+//   FN:<line>,<name>                        function <name> is defined at <line>
+//   FNDA:<hits>,<name>                       function <name> was called <hits> times
+//   DA:<line>,<hits>                         line <line> executed <hits> times
+//   BRDA:<line>,<block>,<branch>,<taken>     branch outcome; <taken> is a count, or "-" if never reached
+//   FNF/FNH · BRF/BRH · LF/LH                found/hit summary counters (RECOMPUTED here on emit)
+//   end_of_record
+//
+// CLI: npx tsx scripts/tools/coverage-shard/mergeLcov.ts <output.info> <input.info|dir ...>
 
 import { readdirSync, statSync, readFileSync, writeFileSync, mkdirSync } from "fs";
 import { dirname, join } from "path";
 
+/** Accumulated coverage for ONE source file. Maps are keyed so records across shards sum. */
 interface FileCoverage {
   testName: string;
-  functionLine: Map<string, number>; // fn name -> definition line (FN)
-  functionHits: Map<string, number>; // fn name -> summed hits (FNDA)
-  lineHits: Map<number, number>; // line -> summed hits (DA)
-  branchTaken: Map<string, number | "-">; // "line,block,branch" -> summed taken, or "-" if never evaluated
+  functionDefLine: Map<string, number>; // FN:   function name -> definition line
+  functionHits: Map<string, number>; // FNDA: function name -> summed call count
+  lineHits: Map<number, number>; // DA:   line number -> summed execution count
+  branchTaken: Map<string, number | "-">; // BRDA: "line,block,branch" -> summed taken, or "-" if unreached
 }
 
-function collectInputs(paths: string[]): string[] {
-  const files: string[] = [];
-  for (const p of paths) {
-    if (statSync(p).isDirectory()) {
-      for (const entry of readdirSync(p)) files.push(...collectInputs([join(p, entry)]));
-    } else if (p.endsWith(".info")) {
-      files.push(p);
-    }
-  }
-  return files;
+type CoverageByFile = Map<string, FileCoverage>;
+
+function newFileCoverage(testName: string): FileCoverage {
+  return {
+    testName,
+    functionDefLine: new Map(),
+    functionHits: new Map(),
+    lineHits: new Map(),
+    branchTaken: new Map(),
+  };
 }
 
-function mergeInto(coverage: Map<string, FileCoverage>, lcov: string): void {
+/** Parse one LCOV report's text and sum its records into `acc` (the running union across shards). */
+function accumulate(acc: CoverageByFile, lcovText: string): void {
+  // `TN:` precedes `SF:`; hold its value until SF: names the file the block belongs to.
+  let pendingTestName = "";
   let current: FileCoverage | undefined;
-  for (const raw of lcov.split("\n")) {
+
+  for (const raw of lcovText.split("\n")) {
     const line = raw.trim();
+
     if (line.startsWith("TN:")) {
-      // Deferred until SF: associates the record with a file.
-      current = {
-        testName: line.slice(3),
-        functionLine: new Map(),
-        functionHits: new Map(),
-        lineHits: new Map(),
-        branchTaken: new Map(),
-      } as FileCoverage;
+      pendingTestName = line.slice(3);
     } else if (line.startsWith("SF:")) {
-      const sf = line.slice(3);
-      if (!coverage.has(sf)) {
-        coverage.set(sf, {
-          testName: current?.testName ?? "",
-          functionLine: new Map(),
-          functionHits: new Map(),
-          lineHits: new Map(),
-          branchTaken: new Map(),
-        });
+      const sourceFile = line.slice(3);
+      current = acc.get(sourceFile);
+      if (!current) {
+        current = newFileCoverage(pendingTestName);
+        acc.set(sourceFile, current);
       }
-      current = coverage.get(sf);
     } else if (!current) {
-      continue;
+      continue; // records before the first SF: have no file to attach to
     } else if (line.startsWith("FN:")) {
       const [defLine, ...nameParts] = line.slice(3).split(",");
-      const name = nameParts.join(",");
-      current.functionLine.set(name, Number(defLine));
+      current.functionDefLine.set(nameParts.join(","), Number(defLine));
     } else if (line.startsWith("FNDA:")) {
       const [hits, ...nameParts] = line.slice(5).split(",");
       const name = nameParts.join(",");
@@ -77,57 +83,90 @@ function mergeInto(coverage: Map<string, FileCoverage>, lcov: string): void {
     } else if (line.startsWith("BRDA:")) {
       const [ln, block, branch, taken] = line.slice(5).split(",");
       const key = `${ln},${block},${branch}`;
-      const prev = current.branchTaken.get(key);
+      const previous = current.branchTaken.get(key);
       if (taken === "-") {
-        if (prev === undefined) current.branchTaken.set(key, "-");
+        // Only record "unreached" if no shard ever reached this branch.
+        if (previous === undefined) current.branchTaken.set(key, "-");
       } else {
-        const base = typeof prev === "number" ? prev : 0;
+        const base = typeof previous === "number" ? previous : 0;
         current.branchTaken.set(key, base + Number(taken));
       }
     }
   }
 }
 
-function emit(coverage: Map<string, FileCoverage>): string {
+/** Render merged coverage back to LCOV text, recomputing the found/hit (`*F`/`*H`) counters. */
+function render(acc: CoverageByFile): string {
   const out: string[] = [];
-  for (const [sf, fc] of coverage) {
+  for (const [sourceFile, fc] of acc) {
     out.push(`TN:${fc.testName}`);
-    out.push(`SF:${sf}`);
-    for (const [name, defLine] of fc.functionLine) out.push(`FN:${defLine},${name}`);
+    out.push(`SF:${sourceFile}`);
+
+    for (const [name, defLine] of fc.functionDefLine) out.push(`FN:${defLine},${name}`);
     for (const [name, hits] of fc.functionHits) out.push(`FNDA:${hits},${name}`);
-    const fnHit = [...fc.functionHits.values()].filter((h) => h > 0).length;
-    out.push(`FNF:${fc.functionLine.size}`);
-    out.push(`FNH:${fnHit}`);
-    for (const [key, taken] of fc.branchTaken) out.push(`BRDA:${key},${taken === "-" ? "-" : taken}`);
-    const brFound = fc.branchTaken.size;
-    const brHit = [...fc.branchTaken.values()].filter((t) => typeof t === "number" && t > 0).length;
-    out.push(`BRF:${brFound}`);
-    out.push(`BRH:${brHit}`);
+    out.push(`FNF:${fc.functionDefLine.size}`);
+    out.push(`FNH:${countHit(fc.functionHits.values())}`);
+
+    for (const [key, taken] of fc.branchTaken) out.push(`BRDA:${key},${taken}`);
+    out.push(`BRF:${fc.branchTaken.size}`);
+    out.push(`BRH:${[...fc.branchTaken.values()].filter((t) => typeof t === "number" && t > 0).length}`);
+
     for (const [ln, hits] of [...fc.lineHits.entries()].sort((a, b) => a[0] - b[0])) out.push(`DA:${ln},${hits}`);
-    const lHit = [...fc.lineHits.values()].filter((h) => h > 0).length;
     out.push(`LF:${fc.lineHits.size}`);
-    out.push(`LH:${lHit}`);
+    out.push(`LH:${countHit(fc.lineHits.values())}`);
+
     out.push("end_of_record");
   }
   return out.join("\n") + "\n";
 }
 
+/** Number of entries with a positive hit count (the `*H` half of an LCOV found/hit pair). */
+function countHit(hits: Iterable<number>): number {
+  let n = 0;
+  for (const h of hits) if (h > 0) n++;
+  return n;
+}
+
+/**
+ * Merge several LCOV report CONTENTS (not file paths) into one LCOV text. Pure and order-independent
+ * over the union — the unit-test entry point.
+ */
+export function mergeLcovReports(reports: string[]): string {
+  const acc: CoverageByFile = new Map();
+  for (const text of reports) accumulate(acc, text);
+  return render(acc);
+}
+
+/** Recursively collect `*.info` files under the given paths (a path may be a file or a directory). */
+function collectInfoFiles(paths: string[]): string[] {
+  const files: string[] = [];
+  for (const p of paths) {
+    if (statSync(p).isDirectory()) {
+      for (const entry of readdirSync(p)) files.push(...collectInfoFiles([join(p, entry)]));
+    } else if (p.endsWith(".info")) {
+      files.push(p);
+    }
+  }
+  return files;
+}
+
 function main(): void {
   const [output, ...inputs] = process.argv.slice(2);
   if (!output || inputs.length === 0) {
-    process.stderr.write("usage: mergeLcov.ts <output.info> <input.info|dir...>\n");
+    process.stderr.write("usage: mergeLcov.ts <output.info> <input.info|dir ...>\n");
     process.exit(1);
   }
-  const files = collectInputs(inputs);
+  const files = collectInfoFiles(inputs);
   if (files.length === 0) {
     process.stderr.write(`mergeLcov: no .info files found under ${inputs.join(", ")}\n`);
     process.exit(1);
   }
-  const coverage = new Map<string, FileCoverage>();
-  for (const file of files) mergeInto(coverage, readFileSync(file, "utf8"));
+  const merged = mergeLcovReports(files.map((f) => readFileSync(f, "utf8")));
   mkdirSync(dirname(output), { recursive: true });
-  writeFileSync(output, emit(coverage));
-  process.stdout.write(`mergeLcov: merged ${files.length} reports → ${output} (${coverage.size} files)\n`);
+  writeFileSync(output, merged);
+  const fileCount = (merged.match(/^SF:/gm) || []).length;
+  process.stdout.write(`mergeLcov: merged ${files.length} reports → ${output} (${fileCount} files)\n`);
 }
 
-main();
+// Run only when invoked as a CLI; importing the module (e.g. from the unit test) does not run main.
+if (require.main === module) main();