Skip to content

test: declare the identity sweep's skips instead of letting them shrink it - #818

Merged
sroussey merged 2 commits into
claude/coverage-dist-bundle-fix-ew0vj8from
claude/optimistic-goldberg-d74u5n-published-entry-guards
Aug 16, 2026
Merged

test: declare the identity sweep's skips instead of letting them shrink it#818
sroussey merged 2 commits into
claude/coverage-dist-bundle-fix-ew0vj8from
claude/optimistic-goldberg-d74u5n-published-entry-guards

Conversation

@sroussey

Copy link
Copy Markdown
Collaborator

Stacked fix onto #741 (claude/coverage-dist-bundle-fix-ew0vj8), which is the base of this PR — not main.

Stacked on top of #817, which it depends on: the two-sided base-class assertion below reads the canonical WORKGLOW_TEST_TARGET that #817's shared.env hands to the workers. Merge #817 first; until it lands, this PR's diff also shows #817's commit. The changes authored here touch exactly one file (packages/test/src/test/util/PublishedEntryIdentity.test.ts) — zero file overlap with #817.

MEDIUM — the sweep skips the exact regression it exists to catch

In PublishedEntryIdentity.test.ts, a candidate whose ai-runtime module exposes no register*Inline export is pushed onto withoutInlineRegistrar and continued, contributing no assertion at all. The only place that surfaced was a message that prints on failure — i.e. never, on a green run.

Counted on this tree: 16 workspace packages publish both ./ai and ./ai-runtime (anthropic, cactus, chrome-ai, deepseek, google-gemini, huggingface-inference, huggingface-transformers, llamacpp-server, mlx, node-llama-cpp, ollama, openai, openrouter, stable-diffusion-server, tf-mediapipe, xai). Every anti-vacuity bound was > 4. So 11 of 16 providers could drop their registrar and leave the file green.

Measured, not reasoned. Renaming registerAnthropicInline in providers/anthropic/src/ai/registerAnthropicInline.ts and rebuilding that provider:

# built bundle after the rename
matching exports: []

$ WORKGLOW_TEST_TARGET=dist npx vitest run … PublishedEntryIdentity.test.ts
Test Files  1 passed (1)
     Tests  6 passed (6)          # GREEN — @workglow/anthropic silently gone from the sweep

With this PR's assertions and the rename still applied, the same run is RED, naming the package in both new checks:

AssertionError: expected [ '@workglow/anthropic', '@workglow/mlx' ] to deeply equal [ '@workglow/mlx' ]
  a runtime entry exporting no register*Inline is checked by nothing — declare it in
  NO_INLINE_REGISTRAR with the reason, or restore its registrar

AssertionError: expected [ '@workglow/cactus', …(13) ] to deeply equal [ '@workglow/anthropic', …(14) ]
- "@workglow/anthropic"

Test Files  1 failed (1)
     Tests  2 failed | 5 passed (7)

The rename was reverted and the provider rebuilt (matching exports: [ 'registerAnthropicInline' ]).

Changes

  • NO_INLINE_REGISTRAR, declared beside NEEDS_NATIVE_RUNTIME. Verified sole member today: @workglow/mlx, which exports registerMlx — no Inline suffix — because MlxProvider.isAvailable reports false until an mlx-lm runtime is bundled, so there is nothing to register inline. withoutInlineRegistrar.sort() is asserted equal to its keys, so a provider that drops its registrar in a refactor fails instead of silently shrinking the sweep.
  • The exemption-pinning test now covers both maps — either one removes a candidate from the checks — asserting every key still names a real candidate and every reason is longer than a word, with the map name in the assertion label.
  • MINIMUM_RUNTIME_CANDIDATES (12) replaces candidates.length > 4: deliberately below today's 16, since a provider may legitimately be removed, but far enough above zero that a typo in the walk fails rather than yielding a short list. checkable.length becomes an equalitycandidates.length - Object.keys(NEEDS_NATIVE_RUNTIME).length — rather than a second floor, so a candidate falling out of the sweep for any undeclared reason fails.
  • The providers.size > 4 block becomes two statements a shrunken sweep cannot satisfy: the registered package set equals the checkable set minus the declared no-registrar packages, and each registration is checked for providerNames.length >= 1, collected into an offenders array so one no-opping registrar reports itself instead of being averaged away by the fourteen that worked.

Two-sided base-class assertion

expect(new Set(PUBLISHED_BASE_CLASSES).size).toBeLessThanOrEqual(2) is satisfied by 1 or 2, so it could not tell a real dist run from a source run mislabelled as one — nor from a stubbed dist re-exporting src. It is now target-keyed:

  • distexactly 2@workglow/ai builds . and ./worker as separate bun build invocations, so two distinct bundles really loaded;
  • sourceexactly 1 — both specifiers resolve through one underlying module, so the resolution plugin really attached.

This is the in-process proof that the modules under test are bundles rather than src. The variable it reads is the one resolveTestTarget already validated in vitest.config.ts (#817), so === "dist" cannot silently mean "source" for a typo the way an unvalidated read would.

Verification (actual output)

$ npx vitest run --config vitest.config.ts …/PublishedEntryIdentity.test.ts
Test Files  1 passed (1)          # source target: asserts exactly 1 base class
     Tests  7 passed (7)

$ WORKGLOW_TEST_TARGET=dist npx vitest run --config vitest.config.ts …/PublishedEntryIdentity.test.ts
Test Files  1 passed (1)          # dist target: asserts exactly 2
     Tests  7 passed (7)

$ bun run test:vitest:dist
Test Files  537 passed | 3 skipped (540)
     Tests  6273 passed | 71 skipped (6344)

$ bun run typecheck:budget       # typecheck-budget: OK (38 packages within budget)

Generated by Claude Code

@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 71.82% 29671 / 41311
🔵 Statements 70.63% 31711 / 44891
🔵 Functions 72.33% 6136 / 8483
🔵 Branches 61.19% 16011 / 26162
File CoverageNo changed files found.
Generated in workflow #3168 for commit 89e8c86 by the Vitest Coverage Report Action

@sroussey
sroussey force-pushed the claude/coverage-dist-bundle-fix-ew0vj8 branch from 758c0b2 to 27b4958 Compare August 16, 2026 16:34
claude added 2 commits August 16, 2026 16:35
…st target

Every vitest project now attaches the source-resolving plugin unconditionally,
so under the default target nothing resolves `@workglow/*` through `exports`.
`publish-all`'s test step therefore stopped touching the bundles it is about to
version-bump and push, and `WORKGLOW_TEST_TARGET=dist` existed only as an inline
`env:` block on one CI job.

- `test:vitest:dist` is the one definition of the dist target, unit tier only
  (matching the CI job; the other tiers want keys, databases and model
  downloads). The variable is set INSIDE the script, so dropping it now means
  deleting the script call.
- `publish-all` runs it after `rebuild`/`format` and before `bunset`.
- The `test-vitest-dist` job invokes the script instead of restating the
  variable.

The dist run also had no precondition. `use-source` writes stubs carrying
`SOURCE_STUB_SENTINEL` and nothing read it, so under a stubbed dist
`@workglow/ai` and `@workglow/ai/worker` collapse onto one source module: every
cross-entry `instanceof` succeeds trivially and export-name parity compares a
file with itself. `assertNoSourceStubs` now runs at CONFIG LOAD under the dist
target, which kills the whole run before any suite can pass vacuously.
`containsSourceStubSentinel` is the node-portable half of the existing Bun-only
`isSourceStub`, since vitest.config.ts is loaded by Vite under node.

`shared.env` hands the validated target down to the workers: `packages/test` is
a composite program rooted at ./src and cannot import scripts/lib/*, so
re-deriving it there would reintroduce the silent-comparison bug
`resolveTestTarget` exists to remove.

`scripts/` was in no CI type gate — `typecheck:budget` globs packages|providers
and `typecheck:tests` globs packages/*/tsconfig.test.json — while now holding
vitest.config.ts's resolution logic. `tsconfig.scripts.json` + `typecheck:scripts`
close that, and the first run surfaced four real errors, all fixed here:
three `WorkspacePackage` literals missing the required `publishes`, a
`TestProjectConfiguration` callback annotation that ignores the union's string
variant, a missing root `vite` devDependency behind
`import type { Plugin } from "vite"`, and a `coverage.root` key that vitest 4
neither types nor reads (its provider derives coverage roots from the resolved
project configs), so it was inert.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lgxtp7mQECdh7F2UT9CVwN
…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
@sroussey
sroussey force-pushed the claude/optimistic-goldberg-d74u5n-published-entry-guards branch from 89e8c86 to 7972f3b Compare August 16, 2026 16:37
@sroussey
sroussey merged commit fd86fd0 into claude/coverage-dist-bundle-fix-ew0vj8 Aug 16, 2026
11 of 12 checks passed
@sroussey
sroussey deleted the claude/optimistic-goldberg-d74u5n-published-entry-guards branch August 16, 2026 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants