ci: fail on unused exports, types and duplicates - #1219
Merged
Merged
Conversation
knip reports four exports that are a second name for another export of
the same module. Keep the declared name and point every caller at it:
- cleanGenerated = cleanRun and materializeRun = materializeSelection:
the CLI clean/materialize commands and the dashboard call the declared
functions, which are also the operation names their audit records
already store ("cleanRun", "materializeSelection").
- findingNoteSchema = findingReportBoundTextSchema: finding notes use
the schema directly. It is the same object, so the generated JSON
Schemas are unchanged.
- MODAL_MAX_SANDBOX_TIMEOUT_MS, MODAL_RECOVERY_SANDBOX_TIMEOUT_MS and
MODAL_SANDBOX_TIMEOUT_MS were one 24-hour value under three names.
MODAL_SANDBOX_TIMEOUT_MS remains, and recovery workers use it. The
layout test loses the two assertions that compared the aliases with
each other.
No value or behaviour changes.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The next commit removes the export keyword from buildPairwiseChart. The diff-limited strict lint (`pnpm -w lint:strict:ci`) applies its budgets to any function whose declaration line changes. buildPairwiseChart is 142 lines long and the budget is 80, so un-exporting it would fail the "Lint changed code strictly" CI step. Move the panel definitions into pairwisePanels() and the drawing of one panel into pairwisePanelParts(). The moved code is unchanged apart from indentation, and the panels still render in the same order. For the same three-pair fixture (including an NA row), paired_row_comparison.svg and .png are byte-identical before and after the split. Co-Authored-By: Claude Opus 5.5 <[email protected]>
knip reports 40 unused exports and 38 unused exported types. Each one is used inside the module that declares it and imported nowhere else: no other module, test, script or package refers to it, and no package entry point re-exports it. Remove the export keyword and keep the declaration. - 69 are in production modules of cli, dashboard, modal, runtime, security and topology; 25 of them are cli-contracts and run-statistics payload types. - 8 are in test helpers of evals, modal and runtime. - 1 is in scripts/ci/verify-cohort-reachability.mjs. assertCurrentPersistentWorkerLineage was async without awaiting anything, which the changed-line strict lint rejects once its declaration line changes. It is now synchronous, and its one caller no longer awaits it. It still throws inside the caller's try/finally while the lineage lock is held, so the lock is still released and the error still rejects the guarded write. The CLI schema registry still looks up its schemas by name in CLI_SCHEMA_EXPORTS, which is now module-private. The temporary-root helper no longer claims that removeRegisteredRoots is exposed. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The previous commits clear every unused export, unused exported type and duplicate export that knip reported. Add those three categories to the --include list of the root knip script, so `pnpm -w knip` (the blocking "Find dead code and dependencies" CI step, and `pnpm -w ci`) now fails on them. Delete the separate continue-on-error "Report unused exports" step, which only printed them. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This was referenced Sep 29, 2026
Merged
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
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.
Problem
pnpm -w knipis the blocking "Find dead code and dependencies" CI step and part ofpnpm -w ci. It checks onlyfiles,dependencies,unlisted,unresolved. A separatecontinue-on-errorstep ranknip --include exports,types,duplicates --no-exit-codeand only printed its findings, so a new unused export never failed a build. Onorigin/main(13af837f), with those three categories included, knip 6.33.0 reports 40 unused exports, 38 unused exported types and 4 duplicate exports. The list is the same before and afterpnpm -w build.Root cause
The three categories stayed out of the blocking script until the earlier dead-code removals landed. The findings that remain fall into two groups:
Change
Four commits:
refactor: delete duplicate export aliases. Each alias is deleted, and callers use the declared name.
cleanGenerated(=cleanRun) andmaterializeRun(=materializeSelection) in runtime. The CLIcleanandmaterializecommands and the dashboard now callcleanRunandmaterializeSelection. These are already theoperationnames that their audit records store.findingNoteSchema(=findingReportBoundTextSchema) in artifacts. Finding notes use the schema directly. It is the same object, so the generated JSON Schemas do not change.MODAL_MAX_SANDBOX_TIMEOUT_MS,MODAL_RECOVERY_SANDBOX_TIMEOUT_MSandMODAL_SANDBOX_TIMEOUT_MSin modal were one 24-hour value under three names.MODAL_SANDBOX_TIMEOUT_MSstays, and recovery workers now use it.layout.test.tsdrops the two assertions that compared the aliases with each other. It keeps the 24-hour value and ordering assertions.refactor(cli): split the pairwise chart into panel helpers. Commit 3 un-exports
buildPairwiseChart, which changes its declaration line.pnpm -w lint:strict:cithen appliesmax-lines-per-function(limit 80) to the whole function, which is 142 lines. The panel definitions move topairwisePanels(), and the drawing of one panel moves topairwisePanelParts(). Apart from indentation, the moved code is unchanged.refactor: stop exporting symbols that only their own module uses. This removes
exportfrom all 78 symbols:cli-contracts.tsandrun-statistics.ts.COHORT_REACHABILITY_LANESinscripts/ci/verify-cohort-reachability.mjs.assertCurrentPersistentWorkerLineagewasasyncbut never awaited anything. Once its declaration line changes, strict lint rejects that (require-await). The function is now synchronous, and its one caller no longer awaits it. It still throws inside the caller'stry/finallywhile the lineage lock is held. Thetemporary-root.tsdoc comment no longer saysremoveRegisteredRootsis exposed.ci: fail on unused exports, types and duplicates.
exports,types,duplicatesare added to the--includelist of the rootknipscript, and the non-blocking "Report unused exports" CI step is deleted.The diff is 43 files, +174/−173. The chart split accounts for +76/−53. The rest is
exportremovals, the alias deletions with their caller and test updates, the synchronous lineage assertion (+2/−5), one doc-comment fix and the CI change.The previous push (
3e6e2718) sat on the old wave-1 integration base (a48ad9c3). This push is rebased ontoorigin/main(13af837f). The rebase hit one conflict, inpackages/cli/src/run-statistics.ts: #1200 added"canceled"toNodeStatisticsStatus, and this PR removes that type'sexport. It is resolved astype NodeStatisticsStatus = NodeStatus | "canceled" | "unknown";. Main still produces the same 82 findings, so no hunk was dropped or added.MODAL_SANDBOX_TIMEOUT_MSis kept although it was the alias, because production code and tests already use that name. It now holds the 24-hour literal, andMODAL_MAX_SANDBOX_TIMEOUT_MSandMODAL_RECOVERY_SANDBOX_TIMEOUT_MSare deleted. The value does not change.Deliberately not built
No knip ignore entries,
@public/@internaltags orignoreExportsUsedInFile. Every finding is fixed in code. The two@internalJSDoc tags already inpackages/runtime/src/git-capture-diagnostics.tsstay.workspace-handoff.tsimports both tagged functions, and the gate still exits 0 with the tags removed, so they hide nothing.Exports that only tests import stay exported. Default-mode knip, the mode CI runs, counts test files as consumers, so such an export is not a finding.
knip --productionleaves out tests and the other non-production entries (build scripts and the runtime workflow templates). On this branch it lists 16 more exports that only those files use. Examples are the CLI schema IDs,setOperatorNpmCliForTests, andPROVIDER_SCOPED_SENSITIVE_ENVIRONMENT_CAPABILITY(read only by controller-source.test.ts). Reworking those is a separate change.Exports that a package entry point re-exports stay exempt. Default-mode knip does not report entry-file exports, and that includes everything an entry pulls in through
export *. With the branch built,knip --include exports,types,duplicates --include-entry-exportsstill reports 207 unused exports and 175 unused exported types. For example, onlypackages/config/src/defaults.tsusesDEFAULT_CODEX_MODELandnormalizeEvalConfig. Butpackages/config/src/index.tsre-exports that module withexport *, so the gate does not report them. Narrowing the package entry points is a separate change.This PR does not touch any
schema/*.jsonfile, generated validator or contract description string. It has no CHANGELOG edit.The CLI schema registry's
typescriptExportvalues foroperatorInputJsonSchemaandreportBundleManifestJsonSchemanow name module-private constants. The field is descriptive only:schemaRegistryBundleDigestdoes not hash it, andjson validatedoes not print it.Verification
The base comparisons ran in a separate detached worktree at
origin/main(13af837f), installed withpnpm install --frozen-lockfileand built withpnpm -w build.[email protected] --include files,dependencies,unlisted,unresolved,exports,types,duplicates --treat-config-hints-as-errors) exits 1. It reports Unused exports (40), Unused exported types (38) and Duplicate exports (4), with the same list before and after the build. The 78 symbols that commit 3 un-exports are exactly the 78 base findings (set comparison). On this branch,pnpm -w knipexits 0 both before and afterpnpm -w build. CI runs it before the build.exportremoval makeslint:strict:cifail onbuildPairwiseChartwithmax-lines-per-function(142 > 80). KeepingassertCurrentPersistentWorkerLineageasyncmakes it fail withrequire-await.packages/cli/src/benchmark-analysis/has not changed on main since the old base.buildComparisonChartrenders a fixed fixture of three pairs, one with null credits and F1 (two NA cells). The base and branch builds produce byte-identicalpaired_row_comparison.svg(sha256c7c75edd…) andpaired_row_comparison.png(6c532e2b…).*JsonSchema(s)values of@ultrafuzz/artifactsserialize identically in the base and branch builds.verify-schema-snapshots.mjspasses on both.Gates at
155d5f20, all exit 0:pnpm -w build,pnpm -w lint,pnpm -w format:check,pnpm -w docs:checkpnpm -w knip, before and after the buildCI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/main pnpm -w lint:strict:ci, with the exit code checked directlypnpm --filtertypecheck for the 8 changed packages: artifacts, cli, dashboard, evals, modal, runtime, security and topologytsconfig.test.jsoncompile for artifacts, cli, dashboard, runtime and securityTargeted tests at
155d5f20, all passing:clean.test,materialize.testbenchmark-analysis.testcleanRunandmaterializeSelectioncalls end to end)worker-lineage.test,layout.testverify-cohort-reachability.test,release-validation-lanes.test(parsesci.yml)worker-lineage.testincludes "serializes competing attempt claims and fences the superseded writer", which exercises the rejection path of the now-synchronous assertion. The full suites were not run, because the host is shared. The rest of the diff isexportremovals, which the typechecks, the test compiles and knip cover.pnpm --filter @ultrafuzz/modal... build, which is what the "Test CI policy scripts" step builds before knip runs.Risk
All workspace packages are
"private": true, and no in-repo consumer uses a removed name. Five names disappear from package entry points:cleanGeneratedandmaterializeRun(@ultrafuzz/runtime)findingNoteSchema(@ultrafuzz/artifacts)MODAL_MAX_SANDBOX_TIMEOUT_MSandMODAL_RECOVERY_SANDBOX_TIMEOUT_MS(@ultrafuzz/modal)None of the 78 un-exported symbols was reachable from an entry point. Recovery workers keep the same 24-hour sandbox timeout, now through
MODAL_SANDBOX_TIMEOUT_MS.Once this lands,
pnpm -w knipfails any PR that leaves an export, exported type or alias unused. A deletion that orphans an export must also delete or un-export it. A PR that starts importing one of the 78 now-private symbols must add itsexportback.Parallel branches. I checked each head below two ways. First, the new knip command ran on the head and on its merge-base with main, to find the findings the head adds. Second, the head was trial-merged onto
155d5f20(git -c rerere.enabled=false merge --no-commit).claude/x07-journal-append-lock,fa27e111) and the dynamic-expansion-retry change (claude/retry-withdraws-deferred-prompts,4b1b3a44). Neither shares files with this PR. Both merge cleanly, and knip exits 0 on each result.28ca73a9) merges cleanly, and knip exits 0.52cb2506) needs three fixes:packages/runtime/src/clean.tshas adjacent deletions: refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197 deletesreadPersistedModalExecutionand this PR deletes thecleanGeneratedalias, so keep neither.packages/runtime/test/cloud-worker-harness.tsis a modify/delete conflict, so take the deletion.currentTaskOutputBinding(packages/modal/test/current-artifact-fixtures.ts:243), which the new gate then reports. The function is still used at line 517 of that file, so the fix is to remove itsexport.54f1706d) addsexport function resolveFrogBininpackages/runtime/src/friction-log.ts. Only that module uses it, so the new gate reports it. The fix is to remove theexport. Its merge conflicts exist against main alone.Refs #462.
🤖 Generated with Claude Code
The PR appears safe to merge; no actionable regression was identified.
Summary
The PR makes unused exports, exported types, and duplicate exports blocking knip findings in CI. It removes the existing findings by making module-local symbols private, deleting redundant aliases, and updating their callers.
Reviews (1) · Last reviewed commit: "ci: fail on unused exports, types and du..."