Replace the sharded c8 coverage report with ast-v8-to-istanbul#3600
Replace the sharded c8 coverage report with ast-v8-to-istanbul#3600DylanPiercey wants to merge 2 commits into
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3600 +/- ##
==========================================
- Coverage 94.71% 89.22% -5.49%
==========================================
Files 420 412 -8
Lines 54887 19480 -35407
Branches 4613 3478 -1135
==========================================
- Hits 51986 17381 -34605
+ Misses 2865 1575 -1290
- Partials 36 524 +488 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
WalkthroughReplaces the c8-based coverage reporter with custom V8-to-Istanbul conversion using worker threads, TypeScript stripping, source maps, deterministic merging, and coverage post-processing. Raw coverage output moves to 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
scripts/coverage-report.js (1)
52-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComment blocks exceed the two-line limit.
Each of these helpers is prefaced with a 5–9 line rationale block. Per coding guidelines, comments should be a last resort and two lines or fewer, capturing intent. Consider condensing each to a one- or two-line intent statement and moving the long validation narratives into
agent-feedback/dx.md, where similar findings are already recorded.As per coding guidelines: "Use comments only as a last resort, keep them to two lines or fewer, and use them to capture intent rather than describe existing or removed code."
Also applies to: 84-90, 112-116, 130-134, 162-166, 204-208, 239-244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/coverage-report.js` around lines 52 - 56, Condense the rationale comment blocks surrounding the helpers in scripts/coverage-report.js—including the block describing duplicate source-map conversions and the noted ranges—to no more than two lines each, retaining only their intent. Move the removed validation narratives to agent-feedback/dx.md alongside similar findings, without changing helper behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/runtime-tags/src/__tests__/utils/bundle.ts`:
- Around line 275-279: Shorten the explanatory comment near the footer-based
code append to no more than two lines while preserving its intent: using
renderChunk without a source map would discard accumulated mappings and break
coverage attribution.
In `@scripts/coverage-report.js`:
- Line 303: Reorganize scripts/coverage-report.js so the main() orchestrator and
runWorker() are declared near the top, before the post-processing helpers such
as dedupeFunctions through dropUnmeasurableBranches. Preserve their existing
implementations and behavior; only relocate the function declarations to
establish the required top-down layout.
- Around line 358-373: Update the Worker promise around the message, error, and
exit handlers so a non-zero exit code rejects instead of resolving successfully.
Keep zero-exit workers resolving normally, and ensure the existing worker error
path still rejects with the relevant error.
- Around line 566-580: Update unionByLoc to normalize location values when
constructing the deduplication id: apply the existing column() helper to both
column fields, while using the raw line values directly and removing any
redundant column() calls on lines. This must make Infinity and
JSON-round-tripped null columns produce the same key.
---
Nitpick comments:
In `@scripts/coverage-report.js`:
- Around line 52-56: Condense the rationale comment blocks surrounding the
helpers in scripts/coverage-report.js—including the block describing duplicate
source-map conversions and the noted ranges—to no more than two lines each,
retaining only their intent. Move the removed validation narratives to
agent-feedback/dx.md alongside similar findings, without changing helper
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77241587-ec1c-4519-8ac7-396efed35326
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by**
📒 Files selected for processing (8)
.c8rc.json.github/workflows/ci.yml.gitignoreagent-feedback/dx.mdpackage.jsonpackages/runtime-tags/src/__tests__/utils/bundle.tsscripts/coverage-report.jsscripts/test-coverage.js
💤 Files with no reviewable changes (1)
- .c8rc.json
| }); | ||
| } | ||
|
|
||
| async function main() { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move main() above the post-processing helpers.
main() is the orchestrator but sits after ~250 lines of low-level helpers (dedupeFunctions … dropUnmeasurableBranches). Function declarations hoist, so relocating main() (and runWorker) to the top is behavior-neutral and matches the required layout.
As per coding guidelines: "Organize files and code top-down: public APIs and exports first, orchestration next, and helpers or low-level details last; use function-declaration hoisting."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/coverage-report.js` at line 303, Reorganize
scripts/coverage-report.js so the main() orchestrator and runWorker() are
declared near the top, before the post-processing helpers such as
dedupeFunctions through dropUnmeasurableBranches. Preserve their existing
implementations and behavior; only relocate the function declarations to
establish the required top-down layout.
Source: Coding guidelines
| function unionByLoc(a, b, locOf) { | ||
| const items = new Map(); | ||
| for (const [hits, itemMap] of [a, b]) { | ||
| for (const [key, item] of Object.entries(itemMap)) { | ||
| const loc = locOf(item); | ||
| if (!loc) continue; | ||
| const id = `${loc.start.line}|${loc.start.column}|${loc.end.line}|${loc.end.column}`; | ||
| const seen = items.get(id); | ||
| if (!seen) { | ||
| items.set(id, { count: hits[key], item, loc, id }); | ||
| continue; | ||
| } | ||
| seen.count = addHits(seen.count, hits[key]); | ||
| seen.item = preferItem(seen.item, item); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Merge id doesn't normalize Infinity/null columns, so identical positions can split.
The id is built from raw loc values, but worker maps are JSON round-tripped (JSON.parse(JSON.stringify(map))), which turns an Infinity column into null. A statement ending at column Infinity from a main-thread conversion and the same statement arriving from a worker therefore key as …|Infinity vs …|null and survive as two entries — the same duplicate-entry class this file's dedupe passes exist to undo. column() already exists for exactly this normalization; use it when building the id (and drop the redundant column() calls on line values).
🐛 Proposed fix
const loc = locOf(item);
if (!loc) continue;
- const id = `${loc.start.line}|${loc.start.column}|${loc.end.line}|${loc.end.column}`;
+ const id = `${loc.start.line}|${column(loc.start.column)}|${loc.end.line}|${column(loc.end.column)}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function unionByLoc(a, b, locOf) { | |
| const items = new Map(); | |
| for (const [hits, itemMap] of [a, b]) { | |
| for (const [key, item] of Object.entries(itemMap)) { | |
| const loc = locOf(item); | |
| if (!loc) continue; | |
| const id = `${loc.start.line}|${loc.start.column}|${loc.end.line}|${loc.end.column}`; | |
| const seen = items.get(id); | |
| if (!seen) { | |
| items.set(id, { count: hits[key], item, loc, id }); | |
| continue; | |
| } | |
| seen.count = addHits(seen.count, hits[key]); | |
| seen.item = preferItem(seen.item, item); | |
| } | |
| function unionByLoc(a, b, locOf) { | |
| const items = new Map(); | |
| for (const [hits, itemMap] of [a, b]) { | |
| for (const [key, item] of Object.entries(itemMap)) { | |
| const loc = locOf(item); | |
| if (!loc) continue; | |
| const id = `${loc.start.line}|${column(loc.start.column)}|${loc.end.line}|${column(loc.end.column)}`; | |
| const seen = items.get(id); | |
| if (!seen) { | |
| items.set(id, { count: hits[key], item, loc, id }); | |
| continue; | |
| } | |
| seen.count = addHits(seen.count, hits[key]); | |
| seen.item = preferItem(seen.item, item); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/coverage-report.js` around lines 566 - 580, Update unionByLoc to
normalize location values when constructing the deduplication id: apply the
existing column() helper to both column fields, while using the raw line values
directly and removing any redundant column() calls on lines. This must make
Infinity and JSON-round-tripped null columns produce the same key.
1635ca0 to
a6f4143
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
scripts/coverage-report.js (1)
355-357: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBare
catch {}discards the reason a script was unconvertible.Here and at Lines 292 and 402 the thrown error is dropped, so the final "Skipped N unconvertible script(s)" list gives no way to tell a parse failure from a missing map. Capturing the message alongside the filename costs nothing and makes the tail of the report actionable.
♻️ Suggested change
- } catch { - failed.push(filename); + } catch (err) { + failed.push(`${filename} (${err.message})`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/coverage-report.js` around lines 355 - 357, Update the catch blocks in the coverage-report conversion paths, including the blocks near lines 292, 355, and 402, to capture the thrown error and store its message together with each filename in failed. Preserve the existing skipped-script reporting while making each entry identify the conversion failure reason.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent-feedback/dx.md`:
- Around line 554-568: Clarify the denominator definitions for the 17,188/17,190
accuracy figure and the 17,194 mutually measured lines in the coverage report.
Update the surrounding explanation to state whether the former counts only lines
with execution-verdict comparisons while the latter includes all lines observed
by both tools, or otherwise reconcile the populations so the metrics are
internally consistent.
In `@scripts/coverage-report.js`:
- Around line 1-19: Trim the oversized comments throughout the coverage-report
implementation, including the header and blocks near the identified sections, to
no more than two lines each. Remove narration about replaced pipelines and
preserve only concise comments that explain current intent; move broader
rationale to agent-feedback/dx.md.
- Around line 63-78: Update the function identity construction in the fnMap loop
to include a declaration end line or body start line in addition to meta.name
and the declaration start line. Preserve merging for the same remapped function
while ensuring distinct same-line anonymous callbacks receive separate entries
and remain in the coverage denominator.
- Around line 638-644: Update addHits so a missing or undefined hit value is
treated as an empty array when the other operand is an array, preserving numeric
element-wise addition. Keep the scalar addition path for two non-array values
and ensure mixed array/undefined inputs cannot produce string counts.
---
Nitpick comments:
In `@scripts/coverage-report.js`:
- Around line 355-357: Update the catch blocks in the coverage-report conversion
paths, including the blocks near lines 292, 355, and 402, to capture the thrown
error and store its message together with each filename in failed. Preserve the
existing skipped-script reporting while making each entry identify the
conversion failure reason.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97267a8e-b4b0-4a32-b198-4b6bb1e9aee0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by**
📒 Files selected for processing (18)
.c8rc.json.changeset/compiler-engines-node-22.md.changeset/escape-raw-text-body-once.md.changeset/runtime-class-engines-node-22.md.changeset/static-text-split-walk-step.md.github/workflows/ci.yml.gitignoreagent-feedback/dx.mdpackage.jsonpackages/compiler/CHANGELOG.mdpackages/compiler/package.jsonpackages/runtime-class/CHANGELOG.mdpackages/runtime-class/package.jsonpackages/runtime-tags/CHANGELOG.mdpackages/runtime-tags/package.jsonpackages/runtime-tags/src/__tests__/utils/bundle.tsscripts/coverage-report.jsscripts/test-coverage.js
💤 Files with no reviewable changes (5)
- .c8rc.json
- .changeset/compiler-engines-node-22.md
- .changeset/runtime-class-engines-node-22.md
- .changeset/escape-raw-text-body-once.md
- .changeset/static-text-split-walk-step.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/ci.yml
- .gitignore
- packages/runtime-tags/src/tests/utils/bundle.ts
- package.json
| for (const [key, meta] of Object.entries(data.fnMap)) { | ||
| // Keyed on name plus the declaration line lcov itself reports. Columns | ||
| // drift when a function is remapped through a bundle's source map, so | ||
| // they split entries that should merge; the declaration line survives. | ||
| const decl = meta.decl ?? meta.loc; | ||
| const id = `${meta.name}@${decl?.start?.line ?? meta.line}`; | ||
| const seen = byKey.get(id); | ||
| if (seen !== undefined) { | ||
| f[seen] += data.f[key] ?? 0; | ||
| continue; | ||
| } | ||
| const next = String(byKey.size); | ||
| byKey.set(id, next); | ||
| fnMap[next] = meta; | ||
| f[next] = data.f[key] ?? 0; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Same-line anonymous functions collapse into one entry.
name is "" for most arrows, so multiple callbacks declared on one line (common in .map(...)/.then(...) chains) key identically and get merged — the uncovered twin disappears from the function denominator, understating missing coverage. Adding the declaration end line (or body start line) to the id keeps the merge for remapped twins while separating genuinely distinct functions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/coverage-report.js` around lines 63 - 78, Update the function
identity construction in the fnMap loop to include a declaration end line or
body start line in addition to meta.name and the declaration start line.
Preserve merging for the same remapped function while ensuring distinct
same-line anonymous callbacks receive separate entries and remain in the
coverage denominator.
| function addHits(a, b) { | ||
| if (!Array.isArray(a) || !Array.isArray(b)) return (a ?? 0) + (b ?? 0); | ||
| const out = (a.length >= b.length ? a : b).slice(); | ||
| const shorter = a.length >= b.length ? b : a; | ||
| for (let i = 0; i < shorter.length; i++) out[i] = (out[i] ?? 0) + shorter[i]; | ||
| return out; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Mixed array/undefined hits produce a string instead of a count.
If one side's hit entry is missing (hits[key] === undefined) while the other is a branch counts array, Array.isArray fails for the undefined side and the scalar path runs: [1,0] ?? 0 + 0 → "1,00". That string flows straight into data.b and out to lcov. Elsewhere the file defends with data.b[key] ?? [], so missing entries are considered reachable here too.
🐛 Proposed fix
function addHits(a, b) {
- if (!Array.isArray(a) || !Array.isArray(b)) return (a ?? 0) + (b ?? 0);
+ if (!Array.isArray(a) && !Array.isArray(b)) return (a ?? 0) + (b ?? 0);
+ if (!Array.isArray(a)) a = [];
+ if (!Array.isArray(b)) b = [];
const out = (a.length >= b.length ? a : b).slice();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function addHits(a, b) { | |
| if (!Array.isArray(a) || !Array.isArray(b)) return (a ?? 0) + (b ?? 0); | |
| const out = (a.length >= b.length ? a : b).slice(); | |
| const shorter = a.length >= b.length ? b : a; | |
| for (let i = 0; i < shorter.length; i++) out[i] = (out[i] ?? 0) + shorter[i]; | |
| return out; | |
| } | |
| function addHits(a, b) { | |
| if (!Array.isArray(a) && !Array.isArray(b)) return (a ?? 0) + (b ?? 0); | |
| if (!Array.isArray(a)) a = []; | |
| if (!Array.isArray(b)) b = []; | |
| const out = (a.length >= b.length ? a : b).slice(); | |
| const shorter = a.length >= b.length ? b : a; | |
| for (let i = 0; i < shorter.length; i++) out[i] = (out[i] ?? 0) + shorter[i]; | |
| return out; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/coverage-report.js` around lines 638 - 644, Update addHits so a
missing or undefined hit value is treated as an empty array when the other
operand is an array, preserving numeric element-wise addition. Keep the scalar
addition path for two non-array values and ensure mixed array/undefined inputs
cannot produce string counts.
e4ee8a6 to
e18bde9
Compare
c8 spent ~65s single-threaded on the report and inflated the result: it credited jsdom-context copies of a file, offset by the old CJS wrapper, onto lines that never ran. Sources now run on native type stripping, so V8's offsets point straight at them and each file converts through the same AST-based remapper Vitest uses, with the runtime-tags fixture bundles — the one source-mapped input left, and ~90% of the wall time — remapped across worker threads. Merging thousands of conversions of the same sources needed the attribution fixed alongside it. istanbul credits a range missing from one map with the hits of its nearest enclosing range, which is right for two instrumentations of the same code and wrong across debug and optimize builds: every `if (MARKO_DEBUG)` block an optimize build eliminates inherited the surrounding count, and `html/template.ts`'s never-callable `mount()` reported 723 hits, one per optimize bundle. Merging on exact position only keeps dead code dead. Where the bundler inlines a statement, nothing anchors its line and every copy stays at zero though the mappings point into executed code, so those segments now resolve against the bundle's own V8 ranges. The entry point also sat above the module's `let` declarations, putting the first conversion in each thread into a temporal dead zone and silently dropping it. Verified against independent instrumentation — a counter spliced before every statement in a statement-list position, run over the full suite. The report agrees on 9,774/9,774 lines for runtime-tags, 5,340/5,341 for runtime-class and 2,074/2,075 for compiler, with 2,598 functions and 527 if/else groups at zero disagreements. The two survivors are loop headers V8 emits no range for; `agent-feedback/dx.md` records them, why sibling-order repair cannot decide them, and the measured tradeoff against instrumenting statements instead. The lcov `DA:` hit counts remain approximate — only their zero/nonzero verdict, which every percentage depends on, is trustworthy.
e18bde9 to
0c2cbc8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent-feedback/dx.md`:
- Around line 554-555: Reconcile the runtime-tags coverage totals in the
documented validation results: update the 9,774/9,774 figure or the stated
9,776/9,776 objective so both refer to the same suite revision or population.
Ensure the published investigation contains one consistent total and clearly
identifies any intentional scope difference.
- Around line 628-630: Update the Yuku discussion to either record the exact
package versions and observation date supporting the registry and package-health
claims, or remove those time-sensitive claims while retaining the reproducible
AST compatibility and benchmark findings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e073244-4361-4ad3-9ce3-af8072a95c63
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by**
📒 Files selected for processing (8)
.c8rc.json.github/workflows/ci.yml.gitignoreagent-feedback/dx.mdpackage.jsonpackages/runtime-tags/src/__tests__/utils/bundle.tsscripts/coverage-report.jsscripts/test-coverage.js
💤 Files with no reviewable changes (1)
- .c8rc.json
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/ci.yml
- .gitignore
- package.json
- packages/runtime-tags/src/tests/utils/bundle.ts
| Independent instrumentation (a counter spliced before every statement in a statement-list position, run over the full suite) now puts the report at 9,774/9,774 lines for `runtime-tags`, 5,340/5,341 for `runtime-class` and 2,074/2,075 for `compiler` — 17,188 of 17,190 — with 2,598 functions and 527 if/else groups cross-checked at zero disagreements. Two line-level defects remain, both the same shape: **V8 emits no range for a loop header**, so the converter resolves the header's offset against whatever range does contain it, and that is either the enclosing function (too high) or a coalesced zero block (too low). | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reconcile the coverage totals with the PR objective.
This entry reports runtime-tags agreement as 9,774/9,774, while the PR objective states 9,776/9,776. Clarify whether these are different suite revisions or populations; otherwise update one of the figures so the investigation does not publish contradictory validation results.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent-feedback/dx.md` around lines 554 - 555, Reconcile the runtime-tags
coverage totals in the documented validation results: update the 9,774/9,774
figure or the stated 9,776/9,776 objective so both refer to the same suite
revision or population. Ensure the published investigation contains one
consistent total and clearly identifies any intentional scope difference.
| Evaluated <https://yuku.fyi> as a replacement for `oxc-parser`. It is real and active — a Zig JS/TS toolchain, `yuku-toolchain/yuku`, pushed within days, with prebuilt native bindings for every platform we care about. Note the bare `yuku` npm package is a 44-byte placeholder (`console.log("Welcome to Yuku")`, last published February); the usable packages are `yuku-parser`, `yuku-ast`, `yuku-codegen` and `yuku-analyzer`. | ||
|
|
||
| `yuku-parser` produces an ESTree-shaped AST that `ast-v8-to-istanbul` accepts unchanged: across 40 real fixture bundles the resulting statement, function and branch counts are byte-identical to oxc's. It is also genuinely faster — 0.34s against oxc raw transfer's 0.51s parsing 18.8 MB of `main.mjs`, and 2.66s against 2.85s converting 120 bundles end to end. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
for package in yuku yuku-parser yuku-ast yuku-codegen yuku-analyzer; do
npm view "$package" version time --json
doneRepository: marko-js/marko
Length of output: 9139
Record the exact package versions and date for the Yuku registry claims, or trim them down to the reproducible benchmark findings.
These package-health details will drift without a version pin and observation date.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent-feedback/dx.md` around lines 628 - 630, Update the Yuku discussion to
either record the exact package versions and observation date supporting the
registry and package-health claims, or remove those time-sensitive claims while
retaining the reproducible AST compatibility and benchmark findings.
1 participant
c8 spent ~65s single-threaded on the report and inflated the result — it credited jsdom-context copies of a file, offset by the old CJS wrapper, onto lines that never ran. Sources now run on native type stripping, so V8's offsets point straight at them and each file converts through the same AST-based remapper Vitest uses; the runtime-tags fixture bundles, the one source-mapped input left and ~90% of the wall time, remap across worker threads. The report step is ~16s.
Merging thousands of conversions of the same sources meant fixing the attribution alongside it. istanbul credits a range missing from one map with the hits of its nearest enclosing range, which is right for two instrumentations of the same code and wrong across debug and optimize builds: every
if (MARKO_DEBUG)block an optimize build eliminates inherited the surrounding count, sohtml/template.ts's never-callablemount()reported 723 hits. Merging on exact position only keeps dead code dead. Where the bundler inlines a statement nothing anchors its line, so those mapping segments now resolve against the bundle's own V8 ranges.Checked against independent instrumentation — a counter spliced before every statement in a statement-list position, run over the full suite. The report agrees on 9,776/9,776 lines for runtime-tags, 5,340/5,341 for runtime-class and 2,074/2,075 for compiler, with functions and if/else groups at zero disagreements. The two survivors are loop headers V8 emits no range for;
agent-feedback/dx.mdrecords them, why sibling-order repair cannot decide between them, and why Node's own--experimental-test-coveragecannot replace this (it marks lines with a plain assignment, so one bundle that skipped a branch erases another bundle's hit).The lcov
DA:hit counts stay approximate — only their zero/nonzero verdict, which every percentage depends on, is reliable.Tooling only, so no changeset.