Skip to content

autophagy: dead-code trim 2026-08-14 - #769

Open
philcunliffe wants to merge 1 commit into
masterfrom
autophagy/cleanup-2026-08-14
Open

autophagy: dead-code trim 2026-08-14#769
philcunliffe wants to merge 1 commit into
masterfrom
autophagy/cleanup-2026-08-14

Conversation

@philcunliffe

Copy link
Copy Markdown
Contributor

Autophagy code-cleanup sweep over origin/master (8c08185). Proposal only: a human disposes.

Scope of the sweep: every .js under src/, hypaware-core/ (plugin workspaces + smoke flows + smoke lib), bin/, scripts/, and test/, plus every .d.ts and every non-JS asset in those trees. Checks run: whole-tree module-orphan scan, exported-symbol reachability scan, module-local declaration scan, static-import usage scan, JSDoc @import usage scan, unreachable-statement heuristic, and a commented-out-code grep.

Trim 1: unused fs and path imports

test/plugins/context-graph-activate.test.js:4 and :5

import fs from 'node:fs/promises'
import path from 'node:path'

Neither name is referenced anywhere in the file. The test builds a fake activation ctx from object literals and asserts on what activate() registered; it never touches the filesystem.

Evidence (run from the repo root):

# every occurrence of `fs` or `path` in the file is the import line itself
grep -n '\bfs\b\|\bpath\b' test/plugins/context-graph-activate.test.js
# ->
# 4:import fs from 'node:fs/promises'
# 5:import path from 'node:path'

That is the whole result: two hits, both the imports. No other use in the file, and imports are file-local so no other file can be affected.

Confirmed mechanically over the whole tree by the import-usage scan (parses each static import clause, strips it from the source, and greps the remainder for each bound name). The only two hits in the entire repo were these two lines.

What I deliberately did NOT trim

1. loadPickerDescriptors in src/core/cli/walkthrough.js:2023 - provably unreferenced, but @ref-annotated.

This is the one genuinely dead export the sweep found. Its name appears exactly once in the whole repository, on its own definition line:

grep -rIn --exclude-dir=node_modules --exclude-dir=.git '\bloadPickerDescriptors\b' .
# -> src/core/cli/walkthrough.js:2023:export async function loadPickerDescriptors() {

It is a one-line wrapper over loadPickerCatalog().descriptors, and loadPickerCatalog (defined 10 lines below it) is what every caller now uses. Its JSDoc still claims "the picker prompt options and composePickerConfig's fold both read from it", which is no longer true.

I left it because its JSDoc carries @ref LLP 0130#picker-block, and a live @ref is a signal the construct realizes a documented decision rather than that it is scrap. Removing it means deciding whether LLP 0130's picker-block section still wants this entry point named, which is a design call, not a mechanical one. Flagging it for a human: if LLP 0130 does not require this specific export, it is a clean delete, and the stale JSDoc claim should go with it.

2. Exported constants and helpers that are used only inside their own module.

The exported-symbol scan surfaced ~70 exports with no importer anywhere: buildUnit / unitPathFor / SystemdUnitError (src/core/daemon/linux.js), buildPlist / plistPathFor / realLaunchctl (src/core/daemon/macos.js), planDaemonInstall, daemonLogDir, planClientAssets, detectShadowedPlugins, normalizeAttributes, nsToHrTime, urlToPath, ATTACH_WAIT_DEFAULT_MS, BATCH_BYTE_LIMIT, OAUTH_*, SESSION_CONTEXT_*, PROSPECT_*, mapFinishReason, resolvePollIntervalMs, and the rest.

Every single one is called or read from inside its defining file. None is dead code; at most the export keyword is broader than today's usage. Narrowing an export is a visible API change (and these are seams a test or a future consumer may want), so it is out of scope for a mechanical trim.

3. Unreferenced interface / type declarations in .d.ts files.

Eleven exported type names appear exactly once in the tree (their own declaration): LocalOnlyListFile, FolderAskFile, ClientSyncListFile (src/core/usage-policy/types.d.ts), ConfigMergeResult (src/core/config/types.d.ts), ObservabilityHandle (src/core/observability/types.d.ts), ParsedNeighbors (context-graph), CreateEmbedder (embedder-openai), ShardBuildReport (vector-search), and IngestSignal / IdentityResponse / IdentityBootstrapRequest (central).

Left alone: tsconfig.build.json emits src/ into a published types/ tree, so these are package type surface a downstream consumer can import even with no in-repo reference. Worth a human look as a separate question, not a mechanical trim.

4. firstPartyPluginMetadata in src/core/config/validate.js:61.

Marked @deprecated, but its own doc says it is "retained only for tests and public-API consumers that have not migrated to the catalog path yet", and it does have in-repo references. Deliberate retention, not scrap.

5. Zero orphan modules.

No .js file under src/, hypaware-core/plugins-workspace/, bin/, or scripts/ is unreachable. The orphan scan's only hits were smoke flows under hypaware-core/smoke/flows/ and files under test/ - both discovered by directory walk / by name rather than by import, so those are expected false positives, not dead files. No unused JSDoc @import type names, no unreachable statements (the two heuristic hits in src/core/cache/partition.js and src/core/cache/spool.js are hoisted helper declarations after a return, which are reachable), and no commented-out code blocks anywhere.

Checks

Baseline origin/master (8c08185) was already green in this worktree, and stays green with the change:

  • npm test: 4029 pass, 0 fail, 1 skipped (identical before and after)
  • npm run typecheck: clean before and after

`fs` and `path` are imported in test/plugins/context-graph-activate.test.js
and never referenced anywhere in the file. Import-only change, no test
behavior touched.

Co-Authored-By: Claude <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Review record: PR #769 @ 8b2e9b5

Verdict: clean. No actionable findings. Nothing pushed, nothing changed.

Reviewed in an isolated detached worktree at origin/autophagy/cleanup-2026-08-14 (8b2e9b5), one commit ahead of origin/master (8c08185).

What the diff actually is

One commit, one file, two deletions, zero additions:

 test/plugins/context-graph-activate.test.js | 2 --
 1 file changed, 2 deletions(-)

Removed lines 4 and 5, import fs from 'node:fs/promises' and import path from 'node:path'. Nothing else in the diff.

Reachability: re-ran, not taken on trust

  1. Every occurrence of the two bound names in the file, at the pre-trim revision. grep -n '\bfs\b\|\bpath\b' against git show origin/master:test/plugins/context-graph-activate.test.js returns exactly two hits, lines 4 and 5, both the import statements themselves. A widened pattern (fs\.|path\.|[^a-zA-Z_$]fs[^a-zA-Z_$]|[^a-zA-Z_$]path[^a-zA-Z_$], to catch member access and non-word-boundary uses) returns the same two hits and nothing more. The PR body's claimed search reproduces exactly.
  2. No dynamic escape hatch in the file. grep -nE "require|createRequire|eval|import\(" over the file: empty. There is no path by which those bindings could be reached by string or dynamic dispatch, so the file-local grep is sufficient.
  3. Scope of the blast radius is one file. ES module import bindings are module-local, so no other file can observe the removal. Confirmed nothing else references this test module anyway: grep -rIn 'context-graph-activate' and grep -rIn 'plugins/context-graph-activate' over the tree (excluding node_modules/.git) are both empty. The test is discovered by directory walk (scripts/run-tests.js over root test/**/*.test.js), not by name, so there is no string-keyed registry, smoke-flow name, or plugin-kernel lookup in play here.
  4. No side-effect imports lost. Both removed specifiers are Node builtins (node:fs/promises, node:path). Importing them has no side effect the test could have been relying on.

Scope: nothing beyond dead-code removal

Whole-diff read confirms no renames, no reformatting, no style churn, no refactors, no assertion or test-semantics changes, no dependency changes, no package.json or lockfile touch. The 60 lines of test body, all 24 assertions, and the three @ref annotations (LLP 0214#d2, LLP 0214#d1, LLP 0213#d2) are byte-identical before and after. The test exercises exactly what it did before.

Conventions

  • No semicolons: grep -nE ';\s*$' over the changed file is empty.
  • No em dashes (U+2014): empty in both the changed file and the commit message body.
  • JSDoc types not TypeScript: the file's /** @type {any[]} */ style is untouched, // @ts-check retained.

Behavior check

Ran the changed test at this head: node --test test/plugins/context-graph-activate.test.js passes, 1/1. CI at this head is green across test (22), test (24), typecheck (22), typecheck (24) and llp-check/duplicate-numbers, which is the authoritative gate.

Non-actionable notes for the human disposing of this PR

  • The loadPickerDescriptors flag in the PR body is accurate. I re-ran it independently: grep -rIn --exclude-dir=node_modules --exclude-dir=.git '\bloadPickerDescriptors\b' . returns exactly one hit, src/core/cli/walkthrough.js:2023, its own definition line. It is genuinely unreferenced. The author's reason for leaving it (its JSDoc carries @ref LLP 0130#picker-block, so deleting it is a design call about whether LLP 0130 still wants that entry point named, not a mechanical trim) is the right call for an autophagy sweep. Worth a separate decision, not a blocker here.
  • The other four "did not trim" categories (module-local exports, unreferenced .d.ts interfaces that are published type surface via tsconfig.build.json, the @deprecated-but-referenced firstPartyPluginMetadata, zero orphan modules) are all correctly scoped out of a mechanical trim. Narrowing an export or deleting published type surface is a visible API change, not dead-code removal.

Summary

Two provably unreachable import bindings removed, in one file, with no other change. The trim is mechanically sound and the diff is exactly what an autophagy member should look like. Nothing to fix.

@philcunliffe philcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Aug 14, 2026
@philcunliffe
philcunliffe marked this pull request as ready for review August 14, 2026 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant