Cleanup: drop five Phase 0 placeholder modules and unreferenced imports - #408
Conversation
Mechanically dead code only, proven unreachable by tree-wide search. - Delete five Phase 0 skeleton placeholder module pairs whose real implementations landed elsewhere; nothing in the tree imports them. - Remove unused value imports and unused local constants (corroborated by `tsc --noUnusedLocals`). - Remove two exported constructs with zero references anywhere in the tree, including inside their own file. No behavior change. Tests and typecheck match the pre-change baseline. Co-Authored-By: Claude <noreply@anthropic.com>
|
Fan-in check on this PR re-derived the reachability claims independently, and they hold: the five deleted One judgement call worth your attention, which is not a mechanical question and so is yours rather than neutral's: Those ten files are Phase 0 skeleton placeholders, and each says so in a comment naming what lands there in Phase 2 (config-section registration and validation; the plugin loader, activation lifecycle, and If that scaffolding is deliberate, this part of the PR should be dropped and the trims narrowed to the unused imports, locals, and unreferenced exports. If the phased plan has moved on and the placeholders are stale, the deletion is clean. Generated by neutral's code-cleanup initiative (LLP 0036). Proposed, not asserted. |
Review round 1 -
|
| Command | Result |
|---|---|
npm run typecheck |
exit 0 |
npm test (head) |
2666 tests, 2657 pass, 8 fail, 1 skipped |
npm test (baseline origin/master) |
2666 tests, 2657 pass, 8 fail - byte-identical failure list |
npm run smoke -- local_parquet_export |
ok (touched by the HERE/fileURLToPath removal) |
npm run smoke -- otel_loopback_capture |
ok (touched by the Attr removal) |
npm run build:types on both revisions |
exit 0 both; diff as above |
All 8 failures are the known pre-existing ones in test/core/leave-command.test.js (tests 778-786), identical before and after, and it remains the only failing file. CI on this PR is green on fresh dependencies for Node 22 and 24, which also settles the authoring pass's caveat about having run against symlinked node_modules.
Still a judgement call for the human
The mechanical case is airtight, but see the note above on this PR: five of the deletions are Phase 0 skeleton placeholders whose comments name what lands there in Phase 2. Removing them is behaviorally inert and, as shown, invisible to the published surface, but it does remove design intent from the tree. Whether that scaffolding is deliberate or stale is not a question neutral can answer from git, and it is the one thing worth a human's attention here.
Held for a maintainer. neutral does not merge.
Mechanically dead code only. Every trim below is backed by a tree-wide search
that came back empty. Nothing here changes behavior, formatting, or naming.
Method
Three independent detectors, then a manual reachability check on each survivor:
.jsand
.tsfile undersrc/,hypaware-core/,bin/,test/,scripts/(including dynamic
import()and JSDoc@importspecifiers) and reportsfiles no specifier ever targets.
tsc -p tsconfig.json --noEmit --noUnusedLocalsfor unused imports andunused local bindings.
tsc -p tsconfig.json --noEmit --allowUnreachableCode falsefor statementsafter
return/throw. It reported nothing, so no unreachable code isremoved here.
Every candidate was then re-checked by hand against: importers anywhere in the
tree, test references,
@ref LLPannotations,package.jsonmain/exports/bin, CLI verbs reachable frombin/, prose references inREADME.md/CONTEXT.md/AGENTS.md/llp//docs//notes-archive//
SKILL.md, and string-keyed dispatch (each symbol was searched as a barestring, not only as an identifier). There is exactly one
package.jsonin thisrepo; the plugin workspaces under
hypaware-core/plugins-workspace/aremanifest-driven (
hypaware.plugin.json), and every dynamicimport()in thetree uses either a literal specifier or a manifest-derived plugin entrypoint,
never a computed core path.
1. Five Phase 0 skeleton placeholder modules (10 files)
Deleted:
src/core/registry/config.js(6 lines)export {}src/core/registry/config.d.ts(17 lines)export type { ... } from '../../../hypaware-plugin-kernel-types.d.ts'src/core/registry/queries.js(6 lines)export {}src/core/registry/queries.d.ts(16 lines)src/core/registry/skills.js(6 lines)export {}src/core/registry/skills.d.ts(6 lines)src/core/runtime/permissions.js(6 lines)export {}src/core/runtime/permissions.d.ts(4 lines)src/core/runtime/plugin_module.js(7 lines)export {}src/core/runtime/plugin_module.d.ts(7 lines)Each
.jsfile's entire body is a stale comment plusexport {}. Example:They date to
5016eae feat(core): observability skeleton + core_boot_noop smoke (hy-83a). The work they reserve a slot for landed elsewhere longago. The config registry is
createConfigRegistryinsrc/core/config/schema.js, the dataset/query registry issrc/core/registry/datasets.js, the skills registry is wired throughctx.skillsfrom the kernel runtime, and plugin activation lives insrc/core/runtime/loader.js+src/core/runtime/activation.js. The.d.tssiblings only re-export types that every real consumer already imports directly
from the root kernel contract,
hypaware-plugin-kernel-types.js.Searches, all empty (run from the worktree root, over the whole tracked tree):
The module-graph pass independently confirms all ten files are targeted by zero
import specifiers. The only other files it flagged as untargeted are legitimate
entry points:
bin/hypaware.js,hypaware-core/smoke/index.js, thepackage.jsonexportssubpaths (src/core/index.js,src/core/query/index.js,src/core/sinks/index.js, ...), and each plugin'smanifest
main(hypaware-core/plugins-workspace/*/src/index.js) - none ofwhich are touched.
None of the ten paths appear in
package.jsonmain/exports/bin(theexportsmap lists.,./core,./core/observability,./core/sinks,./core/query,./core/util,./integration,./tui, and anexportsmapblocks unlisted deep subpath imports for published consumers). None appear in
any LLP document,
README.md,CONTEXT.md,AGENTS.md,docs/,notes-archive/, or anySKILL.md.Note for the reviewer: the
.d.tshalves are the more debatable half ofthis trim. They are also zero-importer, and a
.d.tswith no sibling.jsisstranger than neither, so they go together. If you would rather keep the type
barrels, dropping the five
.d.tsfiles from this PR leaves the rest intact.2. Unused imports
All confirmed by
tsc --noUnusedLocals(TS6133) and by agrep -wshowingthe name occurs exactly once in its file, on the import line itself.
src/core/cache/migrate.js:13import { datasetsRoot } from './paths.js'src/core/cache/retention.js:18import { datasetsRoot } from './paths.js'src/core/commands/status.js:3import path from 'node:path'src/core/daemon/status.js:3import fs from 'node:fs'src/core/remote/credentials.js:6import process from 'node:process'src/core/runtime/boot.js:24defaultBundledWorkspaceDirfrom the./bundled.jsimport listhypaware-core/plugins-workspace/claude/src/projector.js:34defaultSessionContextFilefrom the./session_context.jsimport listhypaware-core/smoke/flows/otel_loopback_capture.js:8Attrfrom the observability import listtest/core/cache-iceberg-schema-evolution.test.js:11import fsSync from 'node:fs'test/core/sink-materialize.test.js:8import fs from 'node:fs/promises'test/core/streaming-reader.test.js:10BATCH_BYTE_LIMITfrom thestreaming-reader.jsimport listtest/plugins/iceberg-commit.test.js:8import { Readable } from 'node:stream'Why safe:
./paths.jstwice, plusnode:path,node:fstwice,node:process,node:fs/promises, andnode:stream).src/core/cache/paths.jshas no top-level side effects (it isimport path from 'node:path', one private const, and three pure exportedfunctions), and the Node builtins have none either, so dropping the statement
cannot change evaluation order or behavior.
are still used, so the module itself is still imported.
@importwas orphaned. In particularsrc/core/daemon/status.jsstill carries
@import { Dirent } from 'node:fs'on its own line, which is aseparate declaration from the deleted value import and is untouched.
keeps at least one live reference; only the unused import site goes away.
datasetsRootis still imported bysrc/core/cache/maintenance.js,src/core/cache/partition.js, andsrc/core/sinks/watermarks.js;defaultSessionContextFilebyplugins-workspace/claude/src/index.jsand twotests;
Attrby many kernel modules.defaultBundledWorkspaceDirandBATCH_BYTE_LIMITstay exported and are still referenced inside their owndefining modules (
src/core/runtime/bundled.js:123andsrc/core/cache/streaming-reader.js:74).3. Unused local constants
hypaware-core/smoke/flows/local_parquet_export.js:15const HERE = path.dirname(fileURLToPath(import.meta.url))test/core/cli/tui/runtime.test.js:15const ENV = { NO_COLOR: '1' }grep -w HEREandgrep -w ENVeach return exactly one hit in their file: thedeclaration. Both flagged by TS6133. Removing
HEREorphansfileURLToPath, which was imported solely for that line, soimport { fileURLToPath } from 'node:url'goes with it;pathis still used11 times in that flow and stays.
local_parquet_exportis a release-checklistsmoke and it still passes (see below).
4. Two exported constructs with zero references anywhere
src/core/cache/spool.js:18-19-DEFAULT_FLUSH_ROW_CHUNK_SIZEgit grep -n -w DEFAULT_FLUSH_ROW_CHUNK_SIZE -- .returns exactly one hit inthe whole repository: this definition. Not referenced in its own file, no test,
no smoke, no
@ref, no prose. Not reachable by a published consumer either:src/core/cache/spool.jsis not in thepackage.jsonexportsmap and anexportsmap blocks unlisted deep imports. Its own JSDoc records that thestreaming-reader batch limits (
BATCH_ROW_LIMIT/BATCH_BYTE_LIMITinsrc/core/cache/streaming-reader.js) replaced it.src/core/observability/tracer.js:73-76-getActiveProvidergit grep -n -w getActiveProvider -- .returns exactly one hit: thisdefinition. Critically, it is not part of the public observability surface:
src/core/observability/index.jsre-exports by explicit name(
getTracer,getLogger,getMeter,withSpan,runRoot,buildAttrs,Attr,context,ROOT_CONTEXT,SpanStatusCode,getActiveSpan, ...) andgetActiveProvideris absent from that list, so theexport *insrc/core/index.js(which backs both the.and./core/observabilitypackage entry points) never picks it up. The
traceimport it used is stillneeded by
getTraceron the line above and stays. No@ref, no test, nosmoke, no prose reference.
Considered and deliberately NOT trimmed
Recording these so the sweep is auditable, and because "unreferenced" alone was
not enough to make them unarguable:
stopLaunchAgent(src/core/daemon/macos.js:357) andstopSystemdUnit(
src/core/daemon/linux.js:286) - both have zero references tree-wide(
hyp daemon stopgoes through the PID-file path insrc/core/daemon/runtime.js, not through launchctl/systemctl). But they aremembers of a symmetric platform-adapter lifecycle family
(install/uninstall/start/stop/restart/status/isInstalled) that
src/core/daemon/install.jsdispatches over per platform. The right fix maybe to add the missing
stopServiceDaemonwrapper rather than delete thehalves, so this is a design question, not a mechanical one.
getGascityRuntime(
hypaware-core/plugins-workspace/gascity/src/runtime.js:35) - zeroreferences;
setGascityRuntimeandrequireGascityRuntimeare both used.Deleting one third of a deliberate set/get/require accessor trio is an API
judgment call.
@importtype names flagged by TS6196 across the tree.Real, but touching them is a wide, low-value diff over documentation-shaped
code, and each one needs care not to break a neighbouring name on the same
@importline. Left alone.test/core/remote-login-command.test.js:444- an unusederrbinding ina destructure of a test helper's return value. The binding also documents the
helper's shape; not worth the churn.
(64 of them). Only the
exportkeyword is redundant there, andremoving an
exportis an API change, not a deletion.Verification
Run in the worktree, before and after, on the same machine.
npm test39bc895)Identical. All 8 failures are the known pre-existing ones in
test/core/leave-command.test.js, with byte-identical test names before andafter:
test/core/leave-command.test.jsis the only failing file in both runs. No newfailures, no newly passing tests, no change in the skip count.
npm run typecheck- clean, exit 0, before and after.Smokes for the two touched flows - both green:
Diffstat: 26 files changed, 1 insertion(+), 106 deletions(-). The single
insertion is the rewritten
import { installObservability } ...line inotel_loopback_capture.jsafter droppingAttrfrom its list.No em dashes introduced (a U+2014 scan of
git diff HEADis empty), nosemicolons added, no reformatting, no renames, no reordering.
Generated by neutral's code-cleanup initiative (LLP 0036). Proposed, not asserted: every trim above is a claim to check, not a fact to trust.