Skip to content

chore: simplify review follow-ups (memoize sort, cache npm, trim comments)#90

Merged
heznpc merged 1 commit intomainfrom
chore/simplify-review
Apr 19, 2026
Merged

chore: simplify review follow-ups (memoize sort, cache npm, trim comments)#90
heznpc merged 1 commit intomainfrom
chore/simplify-review

Conversation

@heznpc
Copy link
Copy Markdown
Owner

@heznpc heznpc commented Apr 19, 2026

Summary

Three targeted wins from `/simplify`. Net +19/-23 lines. Each item only landed after triaging for value — the review also flagged a CI composite-action extraction and a selector flatten, both skipped as over-engineering vs. real cost.

1. Pre-sort `FLASHCARD_COURSE_MAP` once at module load

`Object.entries(map).sort((a,b) => b[0].length - a[0].length)` was running inside the per-URL course resolver in both content.js:320 and sidebar-chat.js:868. Map has ~30 entries so it's not a real hot path, but a pre-sorted `FLASHCARD_COURSE_SLUGS_SORTED` constant is strictly simpler than recomputing. Added to the ESLint global list alongside `FLASHCARD_COURSE_MAP`.

2. Enable npm cache in CI

`validate`, `build`, `test` all run `npm ci` in parallel. With `cache: 'npm'` on `actions/setup-node@v4`, the 2nd and 3rd jobs hit the warm cache and skip the cold install. Expected saving ~60–120s per PR run.

3. Tighten two comment blocks

  • selectors.js "defensive selectors" preamble said the same thing three times — compressed to 3 lines.
  • constants.js `FLASHCARD_COURSE_MAP` docblock over-explained the longest-match semantics; trimmed to 6 lines with an explicit note that keys are hyphenated slugs, values are camelCase deck names.

Findings that were deliberately skipped

Finding Why skipped
Extract CI setup into composite action / reusable workflow `cache: 'npm'` solves the actual cost (cold installs) with one line; composite action adds a file to maintain for the same payoff
Flatten `li.lesson-modular div.title, .lesson-row div.title, ...` into separate array entries `querySelectorAll` handles comma-alternates fine; speculative hot-path, no evidence of real cost
Extract a `lessonRowVariants()` helper for 2 branches Single line, overkill
Rename CI step "DOM health check (Skilljar selectors)" Cosmetic
Reword "conservative" in selector comment Already trimmed out in the main compression

Verification (local)

  • Tests: 309/309 pass
  • Lint / format / selector health / dicts / bg-sync / glossary / translate validate — all green
  • `npm run build:firefox` + `build:bundle` — both build clean

Test plan

  • CI: `validate` + `test` + `build` jobs all pass and show cache-hit logs from `actions/setup-node`

🤖 Generated with Claude Code

Three targeted wins from /simplify review — net +19/-23 lines.

1. Pre-sort FLASHCARD_COURSE_MAP once at module load.
   `Object.entries(map).sort((a,b) => b[0].length - a[0].length)` was
   running inside each URL→course resolve in content.js and
   sidebar-chat.js. Map has ~30 entries and resolve fires on every SPA
   nav, so it's not a real hot path — but a pre-sorted constant is
   simpler than recomputing, and the registered-global list in the
   ESLint config is the only extra surface.

2. Enable npm cache in CI.
   All three jobs (validate, build, test) run `npm ci` in parallel. With
   `cache: 'npm'` on actions/setup-node@v4, the second and third jobs
   hit the cache and skip the cold install — saves ~60-120s per PR run.

3. Tighten two verbose comment blocks.
   - selectors.js: the "defensive selectors" preamble said the same
     thing three times. Compressed to 3 lines.
   - constants.js: the FLASHCARD_COURSE_MAP docblock over-explained the
     longest-match semantics; trimmed to 6 lines with an explicit note
     that keys are hyphenated slugs and values are camelCase deck names.

No behavior change. Tests 309/309 pass; lint, prettier, selectors,
dicts, sync, glossary, validate, firefox build, bundle build all green
locally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@heznpc heznpc merged commit f30dddd into main Apr 19, 2026
3 checks passed
@heznpc heznpc deleted the chore/simplify-review branch April 19, 2026 11: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