Audit and debloat the codebase - #99
Merged
Merged
Conversation
ee5cd2b added a room environment map built with PMREMGenerator. The accessibility test's fake WebGLRenderer cannot back a real generator, so setup threw, the preview fell into its error state, and 9 tests failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No production module imported api/engine.ts. Its six tests move beside the endpoint modules they exercise and import them directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…m react-router-dom Both apps imported zod through hoisting from other packages. instrument.ts was the only module importing the transitive react-router package. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
knip removed export keywords no module imports; tsc (noUnusedLocals) then named every declaration left unreferenced, which was deleted, repeated until clean. Four exports knip misjudged behind lazy route imports stay. The backup upload-path test now imports safeBackupUploadPath by name so the dependency is visible to the type checker. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Point-in-time notes nothing linked to, a one-run PROVE_IT log, the September workflow-cleanup changelog, and the September dependency triage (tracked in issue 67). Git history keeps them. The production route research stays because code cites it; the 09-04 rehabilitation audit stays as the record of known residual limits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The third candidate in loadShippedPathHintRules resolved to a directory that exists in neither the src nor the dist layout. Docs now link to the shipped rules in server data. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ne transport These three settings cards called fetch with bare relative paths, so they ignored VITE_API_URL/VITE_API_PREFIX, sent no credentials cross-origin, and never invoked the app-wide unauthorized handler when a session expired. They now use engineFetchStream, which keeps each card's own failure message and surfaces the server's detail when present. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
npm run quality (and so Web CI) now runs knip. It runs with --no-gitignore because the root .gitignore excludes build/ and then re-includes src/components/build, which knip does not honor; without the flag it silently skips that directory and misreports its imports. Declares the dependencies tests and scripts imported through hoisting (fflate, esbuild, playwright-core, pg, contracts), drops the unused root fastify devDependency, and un-exports buildWorkflowSignature. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lient modules schemaMigrations and the parts trigger DDL move from db/schema.ts to db/migrations-sqlite.ts; postgresPostInitMigrations moves from db/client-postgres.ts to db/migrations-pg.ts. Table exports stay in the schema modules for drizzle. currentSchemaVersion now lives only in db/schema.ts; the Postgres client imports it instead of a duplicate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…services/job-runner InProcessJobRunner, createJobRunner and their types move verbatim to services/job-runner.ts. routes/jobs.ts keeps only the HTTP and WebSocket routes. Adapters, assistant, MCP and route modules import the runner from its new home. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ASSISTANT_TOOL_SPECS, its AssistantToolSpec type and the manifest selection schema it references move verbatim out of assistant/tools.ts. The tool implementations stay in tools.ts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both adapters constructed them and nothing read them. SaasAuthProvider held the only x-tenant-id header tenant resolution in the codebase, and it was never called. Request tenancy already lives in routes/auth. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…seBody Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…teraction graph Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…om lib/guards Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lper Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… paths Source path resolution returns realpaths. On macOS tmpdir() sits under /var, a symlink to /private/var, so six tests failed locally while passing on Linux CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ImportRulesSaveContext and KitManifestSaveContext were the same registry with names swapped. One BuildSaveFlushProvider now holds both, keyed by kind so a Source id and a Profile id cannot collide, and exports useFlushBuildPageSaves directly. The identical save-status label and retry helpers move to lib/autosaveStatus. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…main folderKeyFromRelativePath and the rule normalizer were copies of packages/domain code. Add ./parts-grouping and ./import-rules subpath exports (the domain root pulls in Node-only modules) and import them. applyStackToggle had no callers outside its tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ontracts productionSend.ts copied the PrinterSendQueue* contract types verbatim. planManifests.ts redeclared ReviewPart with assembled_units, spool_summary and spool_badge, which the server's accepted-plan review rows do send; add them to the contract type and use it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
card, empty-state, table-wrap, data-table, manifest-yes, two-col, warning-list, dialog-card, rules-textarea, qty-display, part-thumb, checkoff-grid/card/card-top/thumb-wrap, progress-ring-* and checkoff-mobile-thumb* appear in no component, test page or index.html. .mono is still used by ImportRulesTree and stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The only caller passed () => {}.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…neHealth No caller passed it and the query owns its own interval. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…w updates optimisticReviewCacheKey was only an alias for it. reviewCache keeps rollbackOptimisticCache, which planReview still uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
loadHealth called ensureEngineRunning, which fetched /health, then fetched it again for the data. It now fetches once and keeps the same "API server is not reachable" error on failure; ensureEngineRunning had no other caller. HelpPage reads data_dir from the health query instead of fetching /health a third time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er its removal POST /assistant/chat has returned 410 since the in-app advisor was removed (GRE-225), but tool-loop.ts and its text-recovery, tool-call parsing, and stack-suggestion helpers stayed alive through their own tests. MCP uses invokeAssistantTool and applyAssistantAction directly, which keep their golden-eval coverage. Also removes the Printables metadata stub and the domain secure-path copy, both test-only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SourceFilePickerCard, SourceDocsSheet, useImportRulesAutosave, BuildSourceGuide, BuildWorkflowNextAction, and three lib helpers had no production importer. Import rules still save through SourceDetailSheet and ShareImportSetupPanel. With import-rule autosave gone, the Build save-flush registry only holds kit-manifest flushes, so it keys by Profile id without a kind. Navigation-guard behavior stays covered by BuildSaveNavigationGuard.test.tsx. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… tests that pinned them Found with knip in production mode (tests excluded as entry points), keeping only exports their own module never used either. Most were leftovers of the removed in-app advisor: its system prompt, feedback scoring, and daily token budget. The rest were an unused sidecar slice call, two path guards, spoolman label helpers, an artifact observer, and a catalog mismatch report. Helpers that live tests use as fixtures (loadKitBundleBytes, appendAssistantHistory) stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…heir tests Endpoint wrappers for routes the UI never requests (the removed advisor chat, manifest builder and templates, source notes, several plan and printer reads) and lib helpers only their own tests imported. The server routes stay available to API and MCP clients. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 499 files, which is 199 over the limit of 300. To get a review, reduce the PR to 300 files or fewer by splitting it into smaller PRs or changing its base branch. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (499)
You can disable this status message by setting the Comment |
Contributor
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_14df0228-2946-4a4c-88c7-d589ae46a3ab) |
Owner
Author
|
Independent pre-merge verification: PASS+NOTES for An independent Codex agent that did not author this PR inspected the cleanup's live-risk paths and ran 68 focused tests. SQLite and PostgreSQL migration arrays match the parent byte for byte; the moved job-runner implementation is unchanged. Removed UI chains had no live parent consumer, and active path-security consumers retain the server helper. No concrete merge blocker was found. Limits: this was a bounded review of the large cleanup. Fresh tests ran on integrated stack head |
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
1 participant
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.
Summary
Audit and cleanup of the whole repo. Behavior is unchanged except for three small fixes listed below. About 13,300 fewer lines (20,255 deleted, 6,945 added, lockfile excluded); about 5,700 of the added lines are code moved into new files unchanged.
npm run qualitypasses, and localnpm testnow exits clean on macOS, which it did not onmain.What changed
Dead code
tsc(noUnusedLocals) reported nothing.tool-loop.tsand its helpers, system prompt, feedback scoring and token budget./assistant/chathas returned 410 since the advisor was removed; MCP usesinvokeAssistantToolandapplyAssistantAction, which keep their tests.api/engine.tsre-export barrel, the unusedRepoSourceandAuthProviderports, a dead path-hints fallback, and two unreferenced scripts.Structure
db/schema.ts(2,859 to 1,189 lines) anddb/client-postgres.ts(2,015 to 194). The job runner moved fromroutes/jobs.tstoservices/job-runner.ts. Assistant tool specs moved toassistant/tool-specs.ts. All moves are line-for-line.lib/guards.ts. Two identical save-flush contexts became one. Web copies of domain helpers and contract types now import the originals. About 170 lines of unused CSS removed.Docs
docs/README.md.Fixes
mainfailed because the test's fake renderer could not back the new reflection map. Six server tests failed on macOS only (/varis a symlink to/private/var). Both fixed in the tests.fetchwith bare paths, so an expired session never reached the app-wide re-login handler. They now use the shared transport. A new test fails on the old code./healthrequests; it now sends one.Guardrail
npm run qualitynow runs knip (npm run lint:dead), which fails on unused exports, files and undeclared dependencies. It runs with--no-gitignorebecause the root.gitignoreexcludesbuild/and re-includessrc/components/build, which knip does not honor.Verification
lint, knip, typecheck,
npm test(contracts 101, domain 141, web 1313, server 2146), workflow smoke, build, runtime and browser tests all pass.Reviewer notes
docs/kit-catalog.yaml(the Voron drop-in example catalog),web/README.md, and the path-sanitizer variants (their rules differ).🤖 Generated with Claude Code
Note
Medium Risk
Very large deletion and module moves could hide missed callers despite quality gates; the few auth/transport fixes touch session handling on settings routes.
Overview
This PR is a repo-wide debloat and hygiene pass (~13k net lines removed) that deletes unused exports, tests, web UI, assistant prompt/loop code, the
api/enginebarrel, and unused server ports (RepoSource,AuthProvider), while moving migration DDL, the job runner, and assistant tool specs into smaller modules and deduplicating shared guards/helpers.Docs drop 14+ stale audit/research/verification notes and Cursor verify artifacts;
docs/README.mdis reindexed (ADRs, new guides), and manifest path-hint links now point atweb/apps/server/src/data/path-hints.yaml.Guardrails:
npm run qualityadds knip (lint:dead) to fail on unused exports/files/deps.Small behavior fixes: backup/API-key/logging settings use the shared API transport so session expiry triggers re-login; health polling uses one
/healthrequest; Preview3D and macOS path tests are repaired.Reviewed by Cursor Bugbot for commit 5f637a9. Bugbot is set up for automated code reviews on this repo. Configure here.