Enable knip unused-exports detection; rename gate to lint:unused (#425)#441
Open
geevensingh wants to merge 1 commit into
Open
Enable knip unused-exports detection; rename gate to lint:unused (#425)#441geevensingh wants to merge 1 commit into
geevensingh wants to merge 1 commit into
Conversation
Flips `rules.exports`, `rules.types`, `rules.nsExports`, `rules.nsTypes` from `off` to `error` in `knip.jsonc` so the existing knip gate also catches unused exports + type exports across the root SPA and api/ workspaces. Renames `lint:unused-deps` -> `lint:unused` (and the CI step + summary row + fix-hint) so the name matches the broader behavior (per the rubber-duck architect-2 finding: deferring the rename on cosmetic-churn grounds violates AGENTS.md §11's "don't default to the minimal change" rule). ## Rubber-duck panel Three-agent panel (skeptic + advocate + architect, parallel) flagged 12 distinct findings: 9 adopted, 3 set aside. Set aside: - skeptic-1 (blocker on entry-graph contamination) - empirically refuted: `git ls-tree origin/main` confirms 132 *.test.ts, 0 *.spec.ts, vitest configs present. The `src/testing/a11y.ts` flagged names are post-#418 leftovers. - skeptic-5 (split bundle by risk profile) - advocate's `ruleSets.ts has both 9 unused exports + 8 unused types` cross-file overlap concrete evidence wins. - architect-4 (block on #417 perf-bench Vitest migration) - pre-flight distribution shows zero perf-bench files; #417 + #435 + #438 have all since landed without affecting the finding distribution. Adopted (major): - TAG / `@public` category removed entirely (skeptic-3 + architect-1). Pre-flight categorization confirmed all 58 findings fit DELETE/INLINE; no `@public` taxonomy work needed. - Script + CI rename to `lint:unused` (architect-2). - TELEMETRY_MESSAGE_IDS pre-classified as INLINE (skeptic-2): public surface is the derived `TelemetryMessageId` type. - Per-file test run after each workspace's edits (skeptic-4). Full transcripts in the session folder (plan.critic-skeptic.md, plan.critic-advocate.md, plan.critic-architect.md). ## Per-file dispositions (58 findings) Final tally: 8 DELETEs, 50 INLINEs, 0 ENTRY-GRAPH-FIX, 0 TEST-COUPLING-ACCEPTED, 0 TAG. Detailed disposition table in the PR description. ## SemVer No bump. Tooling/CI + internal-code-cleanup only. No public API, stored shape, user-visible behavior, deployed artifact, or package.json `main`/`exports` change. Touches `api/src/shared/**` utility exports, not `api/src/functions/**` route handlers. ## Verification - `npm run lint:all`: exit 0 in ~60s (all 13 gates green; knip finds 0 issues with the broader rule scope). - `npx vitest run`: 3206/3208 pass (2 skipped). - `npm --prefix api test`: 474/474 pass. - `npm run build`: clean production build, all expected chunks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enables knip's unused-exports / unused-types / unused-nsExports / unused-nsTypes detection (flipping each from off to error in knip.jsonc) and renames the lint script from lint:unused-deps to lint:unused to reflect the broader scope. It then resolves the resulting 58 findings via 8 deletions and 50 inlinings (dropping the export keyword from declarations consumed only within the same file).
Changes:
- Flip 4 knip rule severities (
exports,types,nsExports,nsTypes) fromofftoerror; update the config header comment. - Rename
lint:unused-depstolint:unusedinpackage.jsonand update CI step ID, outcome env var, summary label, and fix-hint accordingly. - De-export or delete 58 unused symbols across 16 files (api shared modules, core json/telemetry/title-suggester, json-tree, contrast, a11y test helpers).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
knip.jsonc |
Flip exports/types/nsExports/nsTypes severities to error; rewrite header comment; update script name in run instructions. |
package.json |
Rename lint:unused-deps -> lint:unused. |
.github/workflows/ci.yml |
Rename CI step, id, outcome env var, summary label, and fix-hint to "Unused code" / lint:unused. |
api/src/shared/blobs.ts |
Drop export on 8 file-local helpers/constants. |
api/src/shared/ruleSets.ts |
Drop export on 9 names + 8 types; remove unused RuleMatchType alias and trim bottom re-export block to just MAX_RULE_SETS_PER_USER. |
api/src/shared/history.ts |
Inline HistoryAction, DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE, getHistoryContainer; delete unused HISTORY_ACTIONS. |
api/src/shared/users.ts |
Inline getUsersContainer. |
api/src/shared/preferences.ts |
Inline 4 file-local types. |
api/src/shared/auth.ts |
Remove JwtPayload re-export. |
src/app/core/api/models.ts |
Inline HistoryAction. |
src/app/core/json/json-extractor.core.ts |
Inline 3 interfaces and 3 functions. |
src/app/core/telemetry/noise-filter.ts |
Inline 2 types; drop unused import + SUPPRESSED_EVENT_ID. |
src/app/core/telemetry/telemetry-message-ids.ts |
Inline TELEMETRY_MESSAGE_IDS (derived type stays public). |
src/app/core/telemetry/msal-bridge.ts |
Inline MsalBridgeEntry. |
src/app/core/telemetry/sw-registration.ts |
Inline BuildIdentity; delete unused SwEventName. |
src/app/core/title-suggester/types.ts |
Inline SuggestionSource. |
src/app/shared/components/json-tree/build-tree.ts |
Drop JsonValueType from re-export list. |
src/app/shared/components/json-tree/flatten.ts |
Inline FlatItemKind. |
src/app/shared/components/json-tree/formatting-rules-engine.ts |
Inline 3 interfaces. |
src/app/shared/utils/contrast.ts |
Delete unused Theme type. |
src/testing/a11y.ts |
Delete OVERLAY_EXCLUDES; inline runA11yScan + assertNoStrictA11yViolations; update docblocks to drop Karma/Jasmine references. |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://calm-flower-01969880f-441.eastus2.7.azurestaticapps.net |
Preview environment deployed
Limitations:
|
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.
Summary
Closes #425. Enables knip's unused-exports / unused-types / unused-ns-exports
/ unused-ns-types detection (flipping
rules.exports,rules.types,rules.nsExports,rules.nsTypesfromoff->errorinknip.jsonc) and renames the script fromlint:unused-deps->lint:unusedto match the broader behavior.Empirical pre-flight showed 30 unused exports + 28 unused exported
types + 0 ns* findings concentrated in ~12 files. All 58 findings
fit DELETE / INLINE without needing
@publictagging (so thefollow-up
@publicconvention work flagged in the plan is NOTrequired for this PR).
What this adds
knip.jsonc. Header comment updated.lint:unused-deps->lint:unused;CI step "Lint - Unused dependencies" -> "Lint - Unused code";
outcome env var, summary emit label, fix-hint string all updated.
What this removes
exportkeywordfrom declarations whose only consumers live in the same file).
What this deliberately does NOT change
tracks the broader §7 reframe).
doesn't change the existing knip step's exit code.
@publicJSDoc convention introduced. Rubber-duck panelsurfaced this as a one-shot taxonomy decision deserving its own
design discussion (architect-1 + skeptic-3). Pre-flight
confirmed all 58 findings fit DELETE/INLINE without TAG.
Per-file dispositions (58 findings)
api/src/shared/blobs.tsapi/src/shared/ruleSets.tsRuleMatchTypealias DELETE'd as unused-internally tooapi/src/shared/history.tsHISTORY_ACTIONSDELETE'dapi/src/shared/users.tsapi/src/shared/preferences.tsapi/src/shared/auth.tsJwtPayloadre-export DELETE'dsrc/app/core/api/models.tssrc/app/core/json/json-extractor.core.tssrc/app/core/telemetry/noise-filter.tsSUPPRESSED_EVENT_IDsrc/app/core/telemetry/telemetry-message-ids.tssrc/app/core/telemetry/msal-bridge.tssrc/app/core/telemetry/sw-registration.tsSwEventNamesrc/app/core/title-suggester/types.tssrc/app/shared/components/json-tree/**src/app/shared/utils/contrast.tssrc/testing/a11y.tsOVERLAY_EXCLUDES; INLINE 2 helpers (used at L254/L255)Final tally: 8 DELETEs, 50 INLINEs, 0 ENTRY-GRAPH-FIX, 0
TEST-COUPLING-ACCEPTED, 0 TAG.
Verification
npm run lint:allnpx vitest runnpm --prefix api testnpm run buildCoordination
No file-overlap with in-flight PRs. #417 + #435 + #438 (perf-bench
Vitest migration tail) all landed during plan iteration; pre-flight
finding count is identical pre/post.
SemVer
No bump. Tooling/CI + internal-code-cleanup only. Touches
api/src/shared/**utility exports, notapi/src/functions/**route handlers.
New CI gate message (observability)
After this PR, the lint-summary row contributors see for knip
findings is "Unused code" (was "Unused dependencies"). Fix-hint:
Rubber-duck
Three-agent panel (skeptic + advocate + architect, parallel,
2026-05-31). 12 distinct findings: 9 adopted, 3 set aside.
Set-aside findings:
karma.perf.conf.js,src/**/*.perf.ts) off Karma+Jasmine #417 timing) -- perf-bench disjoint from findingsAdoptions caused 4 structural plan revisions:
@publiccategory removed entirelysub-rule
Full transcripts in the session folder
(
plan.critic-skeptic.md,plan.critic-advocate.md,plan.critic-architect.md).Follow-ups
After this PR lands, #425 closes. Remaining open follow-ups from
#431's panel:
Session
bb6ac642-dd57-4c49-aded-df3395b91e3e76f32c00-92fd-4ecf-9232-0fc432d1e119