Add workspace source resolution for accurate coverage reporting - #741
Draft
sroussey wants to merge 14 commits into
Draft
Add workspace source resolution for accurate coverage reporting#741sroussey wants to merge 14 commits into
sroussey wants to merge 14 commits into
Conversation
sroussey
force-pushed
the
claude/coverage-dist-bundle-fix-ew0vj8
branch
from
August 11, 2026 03:08
8d27385 to
ced2825
Compare
Coverage Report
File CoverageNo changed files found. |
This was referenced Aug 13, 2026
`packages/test` reaches everything it exercises by package specifier, which `exports` resolves to `dist/node.js`. Under v8 coverage that bundle is the file instrumented, so executed lines were attributed to `packages/ai/dist/*` and `packages/ai/src/**` read as barely covered — a package scored worse the more of its behavior lived behind its public entry point. Measured on the `ai-model` section: every executed line landed on a dist bundle (task-graph/dist 29.7%, ai/dist 23.8%, util/dist 14.0%) and `packages/ai/src` was absent from the report entirely. The fix is a resolver, not a per-package config. `workspaceSourcePlugin` lets normal resolution run first — so conditional exports still pick the node/browser/bun target — and rewrites only the result, `<pkg>/dist/<entry>.js` to `<pkg>/src/<entry>.ts`. Every package, every subpath export, and every package added later is covered with no list to maintain. Unlike `use-source` it writes nothing into `dist`, so it cannot clobber a build or leave a tree that needs `use-dist` afterwards. `WORKGLOW_TEST_TARGET=dist` opts out; the Bun runner still resolves `exports` natively, so the nightly parity workflow exercises the bundles either way. The coverage denominator is now stated explicitly rather than left to vitest's default of "files loaded during the run" — that default omits the modules no test imports at all, which are exactly the ones a coverage report exists to surface, and makes a package's file list depend on which section CI happened to run. `scripts/workspaceSource.test.ts` fails if any published runtime entry lacks a source counterpart: such an entry keeps resolving to its bundle, and the only symptom is one package's coverage collapsing back onto `dist/*`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016khjNuSErZBHzW3aUw2SP4
sroussey
force-pushed
the
claude/coverage-dist-bundle-fix-ew0vj8
branch
from
August 13, 2026 03:53
ced2825 to
97a0b75
Compare
Resolving @workglow/* to src is what makes the coverage numbers mean anything, but the plugin is attached to every project unconditionally — not only to coverage runs — so with the default in force no vitest job resolves a specifier through `exports` at all. There is also no `bun test` job in the blocking workflow. So after the source-resolution change, nothing that can block a merge loads a built bundle: a `bun build` entry that silently dropped a re-export would reach main and surface only in the nightly Bun parity run, which is explicitly informational, runs on a cron, and excludes six sections. Adds test-vitest-dist: reuses the existing build-output artifact and runs the unit tier with WORKGLOW_TEST_TARGET=dist. It is in cleanup's needs list, since cleanup deletes the artifact it downloads. Scoping the plugin to coverage runs instead would not have worked: scripts/test.ts adds --coverage whenever CI is set, so in CI every run is a coverage run and would still resolve to src. Also skips --coverage for a dist-targeted run. The denominator names package sources, so such a run reported all ~1286 of them at 0% — not a measurement of anything, and it is what lets the new job reuse test:vitest:unit unchanged and produce no fragment for merge-vitest-coverage. The CLAUDE.md and vitest.config.ts notes claimed bundle integrity was covered by the nightly parity run; both now say what actually guards it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H797qbH356jjznKgUax63o
The coverage-flag test spawned the runner with `{...process.env, CI: "1"}` and
let WORKGLOW_TEST_TARGET come from the ambient environment. The new
test-vitest-dist job exports that variable for its whole step, so inside that
job the source-target case inherited `dist` and became a second copy of the
dist case — asserting `--coverage` is present while the runner correctly
omitted it. It failed in the one job it was added to support.
Both cases now state the target explicitly, so the assertions hold whatever the
runner is invoked under.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H797qbH356jjznKgUax63o
`examples` is a first-class workspace group in both workspaceSource.ts and testDiscovery.ts, and all three example packages — @workglow/cli, @workglow/eval and @workglow/web — are published with none marked private. They carry 24 test files between them. The denominator listed only packages/* and providers/*, so their source was rewritten to src, executed by their own tests, and then left out of the measurement entirely: a coverage run from examples/cli reported `All files 0%` with no file rows at all. Pins coverage.root to the config's own directory rather than making the globs absolute. coverage.root is the documented base for include/exclude, and this config is invoked from package directories too (vitest run --config ../../vitest.config.ts), where a repo-relative glob would otherwise match nothing. It also does not depend on whether vitest accepts absolute glob patterns. Excludes examples/*/src/test/**: those dirs hold the example suites plus the occasional non-`.test.` helper the filename rules cannot catch, and counting a test helper is what the adjacent excludes already exist to prevent. Guards the invariant that broke, in workspaceSource.test.ts: every entry in WORKSPACE_GROUPS must be a prefix of some coverage.include glob, read from the actual config so the two cannot drift apart again. A missing group is invisible in a coverage report — it shows a shorter file list, not an error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H797qbH356jjznKgUax63o
The source-redirect plugin rewrites the RESULT of resolution, so resolution itself still goes through the package's `exports`, which point at ./dist/*. With no built entry there, Vite's resolution fails first, `this.resolve` yields nothing, the plugin returns null, and the run dies with a generic `Cannot find package '@workglow/ai/worker' imported from …` that blames the manifest and names neither the plugin nor anything to do about it. Keeps the workspace list as WorkspacePackage rather than bare names, so the owning directory is available at the point resolution fails, and throws a message naming the specifier, the owner, the importer and the remedy. Throwing rather than warning is right: an unresolvable @workglow/* specifier already fails the run, so this replaces a misleading message with an actionable one. The remedy branches on whether the owner's dist holds any built entries, since "never built" and "a new exports subpath was added without rebuilding" call for different actions and the second reads as wrong advice to someone looking at a populated dist. An empty dist directory — what `bun run clean` and `use-dist --no-build` both leave behind — counts as never built. ownerOf and unresolvedWorkspaceMessage are separated out as pure functions because resolveId needs Vite's plugin context to drive and cannot be unit tested; the message was additionally verified end to end by emptying dist and running a suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H797qbH356jjznKgUax63o
…st-job ci: keep a blocking job that exercises the built bundles
…-examples fix(test): count examples/* source in the coverage denominator
…ew0vj8' into tmp-resolve-752 # Conflicts: # scripts/workspaceSource.test.ts
…gnostic fix(test): explain an unresolvable workspace specifier
sroussey
marked this pull request as draft
August 13, 2026 04:42
…pecifier causes (#759) Two guards on the dist-targeted test path. `WORKGLOW_TEST_TARGET` had two independent readers, each treating anything that was not literally "dist" as source, silently. A workflow edited to "dist " or Dist turned test-vitest-dist into a byte-identical rerun of test-vitest-unit: green, a full runner slot spent, no bundle coverage. `resolveTestTarget` is now the single reader — unset and empty mean source, everything else throws naming the value, with no lowercasing and no prefix matching. `unresolvedWorkspaceMessage` offered only "never built" or "stale dist", but resolution fails from the IMPORTER's node_modules: under isolated linking an undeclared workspace dependency, or a subpath absent from the owner's exports, fails while the owner's dist is fully populated. The causes are now ranked, so the built/stale pair is offered last and labelled as the remaining likely cause. Claude-Session: https://claude.ai/code/session_01RomTUtZSTgUbFCYqFs4pcu Co-authored-by: Claude <noreply@anthropic.com>
…bundle-fix-ew0vj8
`resolveId` — the one function the workspace-source plugin exists for — had no test, and nothing asserted the plugin reaches the generated projects. `workspaceSource.test.ts` covered only the pure helpers, and both CI modes pass under either resolution: `test-vitest-unit` and `test-vitest-dist` run the same files and the same assertions, differing only in which files load, so no suite distinguished "resolved to src" from "resolved to dist". The failure that leaves is silent. Hoisting `plugins: [workspaceSourcePlugin(__dirname)]` from the per-project object to the root `defineConfig` — a natural "one instance instead of N" cleanup, and the exact mistake the comment there warns about — makes every project resolve `@workglow/*` through `exports` to dist again. All tests pass, merge-vitest-coverage succeeds, and the only symptom is entry-point behavior reading as uncovered. The same silence covers a regression inside `resolveId`. Extract the hook body into `resolveWorkspaceSourceId(packages, context, source, importer, options)`, which takes the plugin context as a parameter so a recording stub can stand in for Vite; `workspaceSourcePlugin` becomes a one-line adapter. `WorkspaceResolveContext` declares `resolve` with method syntax on purpose — parameter bivariance is what lets Vite's real `PluginContext` satisfy it — and is generic in the result so the adapter's return type stays `ResolvedId`, which is what Vite's `resolveId` hook is declared to return. Both together are what keep the call site cast-free (checked against the installed vite typings, where the non-generic form needs one). New tests: the rewrite, a non-workspace specifier short-circuiting before `this.resolve`, an external resolution left alone, dist output with no source twin left on the built file, the unresolved-specifier diagnostic (a branch with zero execution anywhere before this), and `skipSelf: true` plus verbatim forwarding of the hook's own options. Plus a "plugin attachment" block that re-imports the real `vitest.config.ts` and asserts every project carries `workglow:workspace-source` under the default target and none does under `dist`. Both directions stub `WORKGLOW_TEST_TARGET` explicitly and unstub afterwards: `test-vitest-dist` runs this file with that variable ambient, so a test reading the ambient value would pass in one CI job and fail in the other.
`test-vitest-dist` is the one blocking job that resolves `@workglow/*` through `exports`, but it runs the UNIT tier only — while the source rewrite applies to EVERY vitest job. The integration/rag/provider suites previously loaded the bundles and now load src, so a bundle reachable only from an `.integration.test.ts` file lost its blocking check. Two entries lost every check: `@workglow/openrouter/ai-runtime` and `@workglow/huggingface-inference/ai-runtime`, imported only from provider-api integration files, whose section the nightly Bun parity run also excludes. Concretely: a `bun build` change dropping `registerOpenRouterInline` from `providers/openrouter/dist/ai-runtime.js` leaves the file in place, satisfies the dist-must-exist requirement, passes all of CI, and breaks consumers only after publish. `PublishedEntryImports.test.ts` makes the check total instead of tier-shaped: it enumerates every workspace manifest's `exports`, resolves each subpath under the Node conditions only (`node`/`import`/`default`, walked in declaration order the way Node does, so `types`/`browser`/`bun` are stepped over rather than entered), and dynamically imports each resulting specifier, asserting the module is non-empty. Under `WORKGLOW_TEST_TARGET=dist` that one unit-tier file loads every published bundle; under the default target it costs nothing, since it loads the same source the rest of the suite already does. Adding `workglow` to `packages/test`'s devDependencies is the larger half: it brings the meta-package's own entries and, transitively, the provider bundles those re-export. The enumeration is local rather than shared with `scripts/lib/sourceStubs`: `stubSpecsFor` returns dist targets rather than import specifiers, and `packages/test` is a `composite` project rooted at `./src`, so importing from `scripts/` would put those files in its program and break `build-types`. Anti-vacuity assertions (over 60 entries across over 20 packages, every target `./dist/**.js`) keep a mis-typed walk from passing as a short list, and both exemption maps are staleness-checked against the enumeration. Two exemptions, each with its reason: `@workglow/cli` (uncheckable — an example app `packages/test` does not depend on, so under isolated linking the specifier does not resolve from here at all) and `workglow/auto-bootstrap` (imported, but exempt from the non-empty assertion: it registers providers as a side effect and exports nothing by design). New packages default to checked.
…hxoj5s-coverage-guard test: prove the dist→src rewrite is in effect, and make the dist job total
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a Vite plugin that resolves workspace package imports from built bundles to source files during testing, enabling accurate v8 coverage attribution and eliminating duplicate module identities from mixed import graphs.
Changes
scripts/lib/workspaceSource.ts— New module providing:listWorkspacePackages(): Scans workspace groups (packages/,providers/,examples/) to enumerate all packagesdistToSource(): Maps dist entry points back to their source counterparts (e.g.,packages/ai/dist/node.js→packages/ai/src/node.ts)workspaceSourcePlugin(): Vite resolver plugin that intercepts workspace specifier resolution and rewrites dist paths to source paths after normal resolution completesscripts/workspaceSource.test.ts— Comprehensive test suite ensuring:vitest.config.ts— Integration:workspaceSourcePluginto each test project whenWORKGLOW_TEST_TARGET !== "dist"packages/*/src/**/*.{ts,tsx}andproviders/*/src/**/*.{ts,tsx}rather than relying on vitest's default (which omits untested modules).gitignore— Adds coverage output directories (coverage/,.nyc_output/).claude/CLAUDE.md— Documents the coverage strategy andWORKGLOW_TEST_TARGETenvironment variableImplementation Details
The plugin works by letting normal Vite resolution run first (preserving conditional
exportsbehavior for node/browser/bun targets), then rewriting only the resolved path. This approach requires no per-package configuration and automatically covers all packages and subpath exports.The test suite uses
stubSpecsFor()fromsourceStubs.tsto enumerate the same entry points thatuse-sourcestubs from, ensuring the two mechanisms cannot drift into disagreement about what a package exports.WORKGLOW_TEST_TARGET=distrestores the old behavior for verifying bundle integrity; the Bun runner's nativeexportsresolution provides an additional guard via the nightly parity workflow.https://claude.ai/code/session_016khjNuSErZBHzW3aUw2SP4