Skip to content

ci(typecheck-coverage): the tsc -p <dir>/tsconfig.json coverage branch can never match, and its test hides that #10044

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

scripts/check-typecheck-coverage.ts reports every workspace that declares its own typecheck script but is
not reachable from the root typecheck script. It documents a second, equally valid way for a workspace to be
covered — the root chain typechecking the project file directly — and implements it like this:

// scripts/check-typecheck-coverage.ts:73-78
    // A `tsc -p packages/<x>/tsconfig.json` counts too: the root chain typechecks that project directly
    // without going through the workspace's own script.
    for (const match of body.matchAll(/-p\s+((?:packages|apps)\/[\w.-]+)\//g)) {
      const dir = match[1];
      if (dir) covered.add(dir);
    }

so covered can hold a directory like packages/loopover-contract. But the gap filter only ever compares
against a package name:

// scripts/check-typecheck-coverage.ts:81-83
  return workspacesWithTypecheck
    .filter((workspace) => !covered.has(workspace) && !covered.has(workspace.replace(/^@[\w-]+\//, "")))
    .map((workspace) => ({ workspace, script: "typecheck" }));

and main() only ever passes package names, discarding the directory it already computed:

// scripts/check-typecheck-coverage.ts:110-112
  const declared = workspacesDeclaringTypecheck(root);
  const gaps = findTypecheckGaps(rootManifest.scripts ?? {}, declared.map((entry) => entry.name));

workspacesDeclaringTypecheck returns { name, dir } (line 85 onward) precisely so the directory is
available — and main() throws dir away. For @loopover/contract the candidates checked are
"@loopover/contract" and "contract"; the value in covered is "packages/loopover-contract". They can
never be equal. The whole -p branch is dead in production.

The unit test does not catch this because it feeds an input main() never produces — a directory path in the
workspace-names argument:

// test/unit/check-typecheck-coverage-script.test.ts:35-42
  it("counts a project typechecked directly by path as covered, without its own script being invoked", () => {
    const scripts = {
      typecheck: "npm run typecheck:packages",
      "typecheck:packages": "tsc -p packages/loopover-contract/tsconfig.json --noEmit",
    };
    expect(findTypecheckGaps(scripts, ["packages/loopover-contract"])).toEqual([]);
  });

The consequence is a live false-positive waiting to happen. Root package.json's typecheck:packages is
tsc -p packages/loopover-contract/tsconfig.json --noEmit && tsc -p packages/loopover-mcp/tsconfig.json --noEmit && tsc -p packages/loopover-miner/tsconfig.json --noEmit. None of those three packages declares a
typecheck script today, which is the only reason the check is green. The moment any of them gains one — the
obvious, harmless thing to do — npm run typecheck-coverage:check fails inside test:ci on a workspace the
root chain provably does typecheck. Under one-shot review that red check auto-closes a correct contributor PR.

Requirements

  • findTypecheckGaps must treat a workspace as covered when the root typecheck chain typechecks that
    workspace's project file directly, given the inputs main() actually produces.
  • The workspace-directory information must reach findTypecheckGaps: change its second parameter to accept
    the { name: string; dir: string } shape workspacesDeclaringTypecheck already returns, and have main()
    pass declared unchanged instead of declared.map((entry) => entry.name).
  • TypecheckGap.workspace must keep reporting the package name (e.g. @loopover/ui), not the directory —
    the failure message and its npm --workspace fix instruction depend on it.
  • Every currently-passing case must keep passing: a workspace reached by npm --workspace <name> run typecheck
    (either flag order), a workspace reached transitively through an intermediate script, a workspace whose
    build (not typecheck) is run being reported as a gap, a cyclic script graph terminating, and a missing
    entry script being reported rather than throwing.
  • referencedWorkspaces, referencedRootScripts and workspacesDeclaringTypecheck must not change.
  • After the change, npm run typecheck-coverage:check must still exit 0 on the current tree.

⚠️ Required pattern: fix the existing findTypecheckGaps/main() seam — workspacesDeclaringTypecheck
already returns dir for exactly this purpose (scripts/check-typecheck-coverage.ts:85-104). What does NOT
satisfy this issue: (a) deleting the -p branch instead of making it work, which silently drops a real
coverage route and would report @loopover/contract as a gap the day it declares a typecheck;
(b) adding a second, parallel "project-path coverage" helper alongside the existing one rather than fixing
the data flow; (c) a test-only PR that adds a name-shaped assertion while leaving main() passing names
only; (d) making the filter fuzzy (e.g. substring-matching covered against the workspace name), which
would let packages/loopover-ui-kit mark @loopover/ui covered.

Deliverables

  • findTypecheckGaps(scripts, [{ name: "@loopover/contract", dir: "packages/loopover-contract" }]) with
    scripts = { typecheck: "npm run typecheck:packages", "typecheck:packages": "tsc -p packages/loopover-contract/tsconfig.json --noEmit" } returns [], asserted in
    test/unit/check-typecheck-coverage-script.test.ts.
  • findTypecheckGaps(scripts, [{ name: "@loopover/ui", dir: "apps/loopover-ui" }]) with the same
    scripts returns [{ workspace: "@loopover/ui", script: "typecheck" }] — the reported field is the
    package name, asserted in the same file.
  • main() in scripts/check-typecheck-coverage.ts passes declared (the { name, dir } array) directly
    to findTypecheckGaps; no .map((entry) => entry.name) remains.
  • The existing test at test/unit/check-typecheck-coverage-script.test.ts:35-42 is rewritten to use the
    { name, dir } shape instead of a bare directory string, so it can no longer pass on an input the real
    caller never produces.
  • A regression test in test/unit/check-typecheck-coverage-script.test.ts named for this bug, asserting
    that a workspace covered ONLY by a tsc -p <dir>/tsconfig.json in the chain is not reported when it is
    identified by its scoped package name.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that changes the signature but leaves the old directory-string test in place, or that adds the regression test
without changing main() — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts, packages/loopover-engine/src/**/*.ts, packages/loopover-{miner,mcp}/{lib,bin}/**/*.ts,
packages/loopover-contract/src/**/*.ts and packages/discovery-index/src/**/*.ts. It does not cover
scripts/**, and codecov.yml's ignore: list names scripts/** explicitly — Codecov does not gate the
patch on this change.

The tests are still mandatory and still gating, because the root vitest suite (include: ["test/**/*.test.ts"]) runs them on every PR and a failure is a red required check. The required test file is
test/unit/check-typecheck-coverage-script.test.ts.

Every branch of the rewritten filter needs both arms asserted in that file: covered-by--p-path vs
not-covered-by--p-path; covered-by-npm --workspace-name vs not; and the scoped-name-stripping fallback
(workspace.replace(/^@[\w-]+\//, "")) both hitting and missing. The -p regex's own two outcomes (a body
that matches it and a body that does not) must each be exercised.

Expected Outcome

npm run typecheck-coverage:check correctly recognises a workspace that the root chain typechecks via
tsc -p <dir>/tsconfig.json, instead of relying on none of those packages ever declaring a typecheck
script. Adding a typecheck script to @loopover/contract, @loopover/mcp or @loopover/miner no longer
turns test:ci red for a workspace that is genuinely covered.

Links & Resources

  • scripts/check-typecheck-coverage.ts:73-78 — the -p <dir> branch that populates covered with directories
  • scripts/check-typecheck-coverage.ts:81-83 — the filter that only ever compares package names
  • scripts/check-typecheck-coverage.ts:85-104workspacesDeclaringTypecheck, which already returns dir
  • scripts/check-typecheck-coverage.ts:110-112main(), which discards dir
  • test/unit/check-typecheck-coverage-script.test.ts:35-42 — the test that masks the defect
  • package.json typecheck:packages — the three tsc -p packages/... invocations this branch exists for
  • Generate or guard the remaining hand-maintained lists — three shipped red-main incidents traced to one class #9860 — the epic this checker shipped under ("compute the fact rather than remember it")

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions