Add anti-slop Oxlint rules + run against the current codebase - #8
Merged
Merged
Conversation
SlothSoftworks
force-pushed
the
claude/sloth-archiver-overview-ltlvqc
branch
from
September 5, 2026 23:32
d9ab217 to
2c97c3a
Compare
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
tools/oxlint/anti-slop(15 rules targeting low-evidence TS/JS patterns — chained/unjustified type assertions,unknownleaking across function boundaries, ad hoctypeofnarrowing,Reflect.get/apply, module mocking in tests, etc.). Dropped the Effect-framework-specific rule group since this project doesn't use Effect. Per upstream's own model this is vendored and meant to be maintained locally, not tracked as a live dependency — seetools/oxlint/anti-slop/VENDORED.mdfor provenance and how to re-sync later.oxlint.config.tsand addsnpm run lint:anti-slop.oxlint/@oxlint/pluginsadded as exact-pinned (not caret) dev dependencies at matching versions, per upstream's recommendation to keep them in lockstep.lint/husky pre-commit hook. This is a brand-new, quite opinionated tool nobody's reviewed the fit of yet — adding it as a gate is a separate decision from having it available to run on demand.What running it turned up
306 findings from anti-slop + 5 from Oxlint's own bundled
unicornrules, across 37 files. Before reading that as "306 problems," the honest breakdown:require-safety-comment-for-type-assertion) firing almost entirely on ordinaryascasts in test files typing mock data (vitest.setup.ts,testing/mockData/,*.test.tsx). That's normal test-authoring style, not evidence of anything — this rule wants aSAFETY:comment on literally every non-const assertion, which floods test files by design.no-shape-in-symbol-namesflags the functionreshapeVideoInfo(videoInfo.mjs,main.mjs) purely because it contains the substring "shape" — it's just an ordinary verb here, unrelated to the schema-shape confusion the rule is guarding against.no-known-value-wideningflagsKNOWN_PLATFORM_LABELS(utils.ts) andCOOKIE_BROWSER_LABELS(OptionsScreen.tsx) for being typed as openRecord<string, string>dictionaries — but both are intentionally open (arbitrary hostname/browser key → label, with a documented fallback for anything not listed), not a case of discarding known keys.no-runtime-typeofandno-module-mockingreflect an architectural preference (schema-validated boundaries; DI overvi.mock) that doesn't match this app's actual design (no validation-library boundary layer exists) or test setup (mocking Electron'swindow.electronAPIis central to how the test suite works) — enabling these as hard errors would mean redesigning around the linter rather than the linter catching a real problem.useBulkAddQueue.tsx'sextractDownloadErrorMessage(downloadError: unknown)immediately does(downloadError as { payload?: { message?: string } } | null)— acceptingunknownand then blindly re-asserting a shape is exactly the pattern this tool exists to catch.LibraryVideoDetail.tsx:228-229has the...(condition ? { field } : {})conditional-spread pattern flagged byno-conditional-empty-object-spread— though arguably fine as written (the intent, "leave the field alone unless repaired," reads clearly from context).Bottom line: nothing like fabricated APIs, dead code, or actually-unsafe patterns turned up in production code. What surfaced is mostly this tool's own strict take on type-assertion hygiene colliding with normal test-mock typing, plus a couple of debatable-fit rules for this project's actual architecture — not evidence of a systemic AI-slop problem. Worth keeping the tool around and maybe enabling a subset of these rules deliberately (the two "genuinely worth a look" items above are a reasonable starting point) rather than turning all 15 on as blocking errors as-is.
Test plan
npx tsc -bstill passes with the same single pre-existing, unrelated error it has onmaster(awindow.electronAPImock-shape mismatch inApp.tsx— confirmed viagit stashthat it predates this change).git diff package-lock.jsonis purely additive (21 new1.81.0entries for oxlint + its per-platform optional binaries; no existing package's resolved version changed).npm run lint:anti-slopruns cleanly (exits with findings as documented above, not a crash).🤖 Generated with Claude Code
https://claude.ai/code/session_01Wnz52wfC7mGqEGfrpeBRdY
Generated by Claude Code