Skip to content

refactor: dedupe read_dir loops; drive biome lint to clean#43

Merged
RealZST merged 2 commits intomainfrom
refactor/dedupe-readdir-and-fix-lints
May 8, 2026
Merged

refactor: dedupe read_dir loops; drive biome lint to clean#43
RealZST merged 2 commits intomainfrom
refactor/dedupe-readdir-and-fix-lints

Conversation

@RealZST
Copy link
Copy Markdown
Owner

@RealZST RealZST commented May 8, 2026

Summary

  • Mechanical refactor: 8 pre-existing read_dir + extension filter loops (claude rules/commands/output-styles/projects-memory, gemini commands/policies, copilot hooks, codex memories) replaced with calls to the files_with_ext helper extracted in PR feat: scan project-level subagents across 6 agentsΒ #42. Behaviour-preserving.
  • Frontend lint cleanup: biome diagnostics driven from 13 errors + 12 warnings + 1 info β†’ 0 / 0 / 0. Mix of npm run lint:fix (auto-fix), targeted refactors, and documented biome-ignore for legitimate trigger-sentinel patterns.
  • No behaviour changes; pure tech-debt cleanup.

Commits

f54bfbd chore(lint): drive biome to clean (0 errors / 0 warnings / 0 info)   (+120 / -51, 21 files)
5816098 refactor(adapter): dedupe remaining read_dir loops with files_with_ext (+16 / -74, 4 files)

Each commit is independently bisect-able and tests-clean.

Commit 1 β€” Rust read_dir dedup

Adapter Function Replacement
claude global_rules_files (rules/), global_settings_files (commands/, output-styles/), global_memory_files (projects/*/memory/ inner loop) 4 loops β†’ 4 helper calls
gemini global_settings_files (commands/, policies/) 2 loops β†’ 2 helper calls
copilot global_settings_files (hooks/) 1 loop β†’ 1 helper call
codex global_memory_files (memories/) 1 loop β†’ helper + .filter(|p| p.is_file()) to preserve prior is_file() defensive check

Plugin / marketplace recursive multi-level scans intentionally left untouched β€” they don't fit the files_with_ext shape.

Closes the follow-up tracked in project_adapter_readdir_helper_extract_roadmap.md.

Commit 2 β€” Frontend lint cleanup

Auto-fix (Step 2 β€” npm run lint:fix, 8 files)

Pure cosmetic: import sorting, formatter pass, blank-line cleanup. Zero behaviour change.

Manual fixes by category

Category A β€” trivial mechanical (2 sites)

  • transport.ts useLiteralKeys: headers["Authorization"] β†’ .Authorization
  • invoke.ts useOptionalChain: !value || !value.trim() β†’ !value?.trim()

Category D β€” noExplicitAny (1 site, onboarding.tsx)

  • Removed dead roughness field from rough-notation's annotate() config (library hardcodes its own roughness via getOptions(type) in render.js and never reads user-supplied values), removed accompanying as any.

Category C β€” useExhaustiveDependencies (4 files, 5 findings)
Each is a deliberate trigger-sentinel pattern; auto-fix would have broken behaviour. Documented with single-line biome-ignore:

  • section-anchor-rail revisionKey (re-discovery sentinel β€” see component JSDoc)
  • extension-filters extensions (Zustand store closure trigger via grouped())
  • extension-table scope (cell-renderer getState() trigger)
  • scope-switcher-menu handleSelect / handleAddProject (new closures capturing only stable refs β€” adding them only churns the keydown listener)

Also replaced an obsolete eslint-disable-next-line comment (project uses biome, not eslint).

Category B β€” noNonNullAssertion (9 sites)
Per-site judgment call:

  • 5 biome-ignore where the ! is a TS-narrowing limitation through JSX or by-construction guarantees (React entry pattern, JSX && gates not propagating into callbacks, Map.get of own keys, test idiom).
  • 4 refactors where a small rewrite removes the ! cleanly:
    • delete-dialog: redundant && filteredSkillLocations lets TS narrow inside the ternary's true branch
    • extension-detail: extracted activePath local variable via IIFE wrapper (kills 2 !)
    • new-skills-dialog: explicit Map get-or-create binding instead of has() + get()!
    • agent-config-store: single get() + cached !== undefined check (cache stores strings only, so undefined ≑ !has); also reduces a redundant zustand get() call

Test coverage

  • cargo build --workspace: clean
  • cargo test --workspace: 402 tests pass (385 hk-core + 17 across other crates)
  • cargo clippy --workspace --all-targets -- -D warnings: clean
  • npx tsc --noEmit: clean
  • npm run lint (biome): 0 errors / 0 warnings / 0 info
  • npx vitest run: 148 / 148 pass

Test plan

  • CI passes on all platforms
  • No visible regression in agent detail panels (Subagents/Settings/Workflow/Rules/Memory/Ignore sections)
  • No visible regression in extension detail / delete dialog / new-skills dialog
  • Onboarding rough-notation annotations still render correctly (since roughness user-config was dead, visuals should be identical)

Roadmap follow-ups not in this PR

None new. The roadmap entry project_adapter_readdir_helper_extract_roadmap.md is now complete and can be retired.

RealZST and others added 2 commits May 8, 2026 19:22
Follow-up to PR #42's helper extraction. Replaces 8 pre-existing
`read_dir + extension filter` loops with `super::files_with_ext` helper
calls; behaviour-preserving mechanical refactor, 4 adapters touched:

- claude.rs: rules/, commands/, output-styles/ (3 loops in
  global_rules_files / global_settings_files); inner memory/ loop in
  global_memory_files (outer projects/* iteration kept β€” multi-level
  pattern doesn't match the helper)
- gemini.rs: commands/, policies/ in global_settings_files
- copilot.rs: hooks/ in global_settings_files
- codex.rs: memories/ in global_memory_files β€” adds explicit
  `.filter(|p| p.is_file())` to preserve prior `is_file()` check
  (cheap parity over a "siblings don't bother" argument)

Net: -64 / +12 lines. 385 hk-core tests pass, clippy -D warnings clean.
Plugin/marketplace recursive multi-level scans intentionally left
untouched β€” they have a different shape than the helper.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reduces biome diagnostics from 13 errors + 12 warnings + 1 info to fully
clean. Combines auto-fix and manual passes:

Auto-fix (Step 2 β€” `npm run lint:fix`, 8 files): import sorting,
formatter pass, blank-line cleanup. Pure cosmetic, zero behavior change.

Manual (Step 3, by category):

Category A β€” trivial mechanical (2 sites):
- transport.ts useLiteralKeys: `headers["Authorization"]` β†’ `.Authorization`
- invoke.ts useOptionalChain: `!value || !value.trim()` β†’ `!value?.trim()`

Category D β€” noExplicitAny (1 site, onboarding.tsx):
- Removed dead `roughness` field from rough-notation `annotate()` config
  (library hardcodes its own roughness via getOptions(type) in render.js
  and never reads user-supplied values), removed accompanying `as any`.

Category C β€” useExhaustiveDependencies (4 sites, 5 findings):
- Added single-line `biome-ignore` with rationale at each site where the
  dep array intentionally omits a value or includes a trigger sentinel:
  - section-anchor-rail revisionKey (documented re-discovery sentinel)
  - extension-filters extensions (Zustand getter trigger)
  - extension-table scope (cell-renderer getState() trigger)
  - scope-switcher-menu handleSelect/handleAddProject (new closures
    capturing only stable refs β€” adding them would only churn the
    keydown listener)
- Replaced obsolete `eslint-disable-next-line` comment in
  scope-switcher-menu (project uses biome, not eslint).

Category B β€” noNonNullAssertion (9 sites, 5 biome-ignore + 4 refactor):
- biome-ignore where the `!` reflects a TS narrowing limitation through
  JSX/by-construction guarantees (main.tsx React entry, config-file-entry
  JSX gates, detail-paths Map.get of own keys, test idiom).
- Refactor where a small rewrite removes the `!` cleanly:
  - delete-dialog: redundant `&& filteredSkillLocations` lets TS narrow
  - extension-detail: extracted `activePath` local var (kills 2 `!`)
  - new-skills-dialog: explicit Map get-or-create binding
  - agent-config-store: single `get()` + undefined check instead of
    has()/get()! pair (cache stores strings only, so `cached !== undefined`
    is exactly equivalent to `has`)

Final state: `npm run lint` clean; tsc clean; 148/148 frontend tests
pass; no behavior changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@RealZST RealZST merged commit e0c91d4 into main May 8, 2026
3 checks passed
@RealZST RealZST deleted the refactor/dedupe-readdir-and-fix-lints branch May 8, 2026 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant