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
sroussey
force-pushed
the
claude/coverage-dist-bundle-fix-ew0vj8
branch
from
August 13, 2026 03:53
ced2825 to
97a0b75
Compare
sroussey
marked this pull request as draft
August 13, 2026 04:42
This was referenced Aug 13, 2026
sroussey
force-pushed
the
claude/coverage-dist-bundle-fix-ew0vj8
branch
from
August 16, 2026 16:34
758c0b2 to
27b4958
Compare
Collaborator
Author
|
@claude rebase on main |
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
`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.
Importing every published entry proves each bundle LOADS; it does not prove the bundles agree with each other. Two entries of one package can each resolve, and still hand a consumer two different classes for one symbol — an `instanceof` across them then fails for a reason no import sweep can see. PublishedEntryIdentity pins the cross-entry identities that matter, and PublishedEntryExportParity compares each entry's named exports against its source barrel, so a re-export a `bun build` change dropped is named rather than inferred from a downstream failure. Both re-derive the workspace groups from the root manifest rather than importing the tooling's copy: `packages/test` is a composite project rooted at `./src`, so reaching into `scripts/` would pull those files into its program and break `build-types`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01By1Tpxcr4qNBjauTK1QeNj
`WORKGLOW_TEST_TARGET=dist` existed only as an inline `env:` block on one CI job. Nothing in-process can notice it going missing — an unset target resolves to `source` — so a workflow edit that dropped it would leave a green job that is a duplicate of the source run. `test:vitest:dist` becomes the one definition of the target, the CI job invokes the script, and `publishPipeline.test.ts` pins both, plus the `require-green-ci` step through which `publish-all` reaches that job before anything is versioned or pushed. `scripts/` was in no CI type gate — `typecheck:budget` globs packages|providers and `typecheck:tests` globs packages/*/tsconfig.test.json — while holding the resolution logic that decides what every suite in the repo resolves a `@workglow/*` specifier to. `tsconfig.scripts.json` plus `typecheck:scripts` close that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01By1Tpxcr4qNBjauTK1QeNj
…nk it The sweep skipped the exact regression it exists to catch. A candidate whose `ai-runtime` module exports no `register*Inline` was pushed onto `withoutInlineRegistrar` and `continue`d, contributing NO assertion, and the only place that surfaced was a message printed on failure. Sixteen workspace packages publish both `./ai` and `./ai-runtime`, and every bound was `> 4`, so eleven of the sixteen could drop their registrar and leave the file green. Measured rather than reasoned: renaming `registerAnthropicInline` and rebuilding the provider left all six assertions passing, with `@workglow/anthropic` gone from the sweep and nothing saying so. - `NO_INLINE_REGISTRAR` declares the skips, beside `NEEDS_NATIVE_RUNTIME`. Sole member today is `@workglow/mlx`, whose registrar is `registerMlx` — no `Inline` suffix — because `MlxProvider` stays unavailable until an mlx-lm runtime is bundled. `withoutInlineRegistrar` is compared for EQUALITY against its keys, so an undeclared skip fails. - The exemption-pinning test covers both maps: every key names a real candidate, every reason is longer than a word. - `MINIMUM_RUNTIME_CANDIDATES` replaces `candidates.length > 4`, and `checkable.length` is now an equality against candidates minus the declared native-runtime exemptions rather than a second floor. - The `providers.size > 4` bound becomes two statements that cannot be satisfied by a shrunken sweep: the registered package set EQUALS the checkable set minus the declared no-registrar packages, and each registration is checked for at least one provider name, collected into an offenders array so one no-opping registrar reports itself. - The base-class allowance is now two-sided and target-keyed: `dist` requires exactly 2 distinct classes (two bundles really loaded), `source` exactly 1 (the resolution plugin really attached). `<= 2` was satisfied by either, so it could not tell a real dist run from a source run mislabelled as one. This is the in-process proof that the loaded modules are bundles. With the rename still applied the new file fails, naming `@workglow/anthropic` in both the undeclared-skip check and the registered-package set; reverted and rebuilt, it passes on both targets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lgxtp7mQECdh7F2UT9CVwN
Under the default `source` target the resolving plugin rewrites `import(specifier)` to exactly the path `sourceCounterpart()` computes, so both sides are the SAME module and every case asserts `X === X`. Those cases now skip, so the report distinguishes "checked" from "not applicable" rather than showing ~90 green rows that compared nothing. Nothing stops being loaded: `PublishedEntryImports.test.ts` imports every published specifier unconditionally and does carry signal under source. A skip keyed on a value that can go missing is its own hazard, so the target is handed down validated through `test.env` and an anti-vacuity case pins it to one of the two known values — otherwise broken plumbing would skip every case in every job and take `test-vitest-dist` green having compared nothing. That plumbing is vitest's, so both sweeps carry `@vitest-environment`, which is what keeps them off the Bun runner. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01By1Tpxcr4qNBjauTK1QeNj
sroussey
force-pushed
the
claude/coverage-dist-bundle-fix-ew0vj8
branch
from
September 20, 2026 23:34
0cd71d6 to
ee6ee02
Compare
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