Merged
Conversation
… prod log strip, protected-terms hardening)
Multi-front cleanup from a 2026-05-10 review covering the items the recent
hotfix train (v3.5.6 → v3.5.12) repeatedly touched: long single-purpose
files, missing static type signal, no enforced i18n drift check, and a few
production-realistic edge cases in shared helpers.
Refactor — `src/content/sidebar-chat.js` (1224 → 815 lines, –33%) split into:
* `chat-render.js` (formatResponse / applyInline / sanitizeHtml)
* `chat-history.js` (IDB store + history sub-panel)
* Sub-panel state (savedChatHTML, history/flashcard flags) hoisted to
`_sb._chat.state` so the modules share one source of truth.
* Manifest content_scripts.js order updated; format-response.test.js
follows the functions to chat-render.js.
Added:
* `tsconfig.json` (allowJs + checkJs, strict:false) for IDE / `tsc --noEmit`.
* `src/lib/_sb-typedef.js` — JSDoc-only contract for `window._sb` and the
new `_chat` / ProtectedTermsApi / GeminiBlockApi sub-namespaces.
* `scripts/check-i18n-keys.js` + CI step + `npm run check:i18n` —
validates _locales/ key parity and constants.js label-dict shape across
the 11 premium languages (handles flat, lang-outer, and section-outer
shapes; skips language-agnostic lookups like SKILLBRIDGE_MODEL_LABELS).
* `docs/E2E_PLAN.md` — working spec for the deferred Playwright suite
(6 priority targets) so the next pass doesn't restart from zero.
Fixed:
* `restoreProtectedTerms` defends against null/undefined/non-string input,
empty-string wrong-forms (would have inserted between every char via
`replaceAll('', x)`), and self-mapping entries.
* Background SW + content-script handlers now warn on misrouted
`action`/`type` discriminators (catches the v3.5.6 cache-cleanup class
of bug at first occurrence in dev instead of silent fallthrough).
Build:
* `build-bundle.js` esbuild gains `pure: ['console.debug','console.info']`
so production bundles strip dev-only logs (verified 0 occurrences in
dist/bundled). `console.warn`/`error` preserved on purpose.
Tests: 343/343 (+7 new, +6 in protected-terms covering the new edge cases).
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
Multi-front cleanup from a 2026-05-10 review covering the items the recent hotfix train (v3.5.6 → v3.5.12) repeatedly touched: long single-purpose files, missing static type signal, no enforced i18n drift check, and a few production-realistic edge cases in shared helpers.
Refactor
src/content/sidebar-chat.js(1224 → 815 lines, –33%) split into:chat-render.js(new) —formatResponse/applyInline/sanitizeHtml. Pure DOM-free markdown→HTML + the trusted-structure HTML sanitizer used by both the live chat bubble and history detail view.chat-history.js(new) — IndexedDB conversation store + history sub-panel. Calls back into sidebar-chat through_sb._chat.{closeSubPanel,formatResponse,sanitizeHtml}.savedChatHTML,historyPanelOpen,flashcardPanelOpen) hoisted to_sb._chat.stateso the modules share one source of truth instead of duplicating local flags.manifest.jsoncontent_scripts.jsorder updated;tests/format-response.test.jsfollows the functions to their new home.Added
tsconfig.json—allowJs+checkJsfor IDE /tsc --noEmit.strict:falseso the rollout doesn't surface a wave of pre-existing nullability warnings; tighten incrementally.src/lib/_sb-typedef.js— JSDoc-only contract forwindow._sband the new_chat/ProtectedTermsApi/GeminiBlockApisub-namespaces. Not loaded at runtime (no manifest entry).scripts/check-i18n-keys.js+ CI step +npm run check:i18n— validates (1) every_locales/<lang>/messages.jsonmatches the English key set, and (2) every*_LABELS/*_GREETINGS/*_PLACEHOLDERSdictionary inconstants.jsis shape-consistent across the 11 premium languages. Handles flat, lang-outer, and section-outer shapes; skips language-agnostic lookups likeSKILLBRIDGE_MODEL_LABELS.docs/E2E_PLAN.md— working spec for the deferred Playwright suite. Records the 6 priority coverage targets (golden translation, SPA mid-stream, cache-cleanup alarm, stream-cancel, protected-terms, panel switch) so the next pass doesn't restart from zero.Fixed
restoreProtectedTermsdefends against three production-realistic edge cases: null/undefined/non-string input (returned safe fallback instead of throwing on.includes); empty-string wrong-forms (would have inserted the correct form between every char viareplaceAll('', x)); self-mapping entries where wrong-form === correct-form (silent no-op cycle bloating the hot loop on long pages)._logMisroutedMessagedefensive logs: a{action: ...}-shaped message reaching the background (or{type: ...}reaching a content script) warns loudly with the discriminator. Catches the v3.5.6 cache-cleanup class of bug at first occurrence in dev instead of silent fallthrough to "Unknown action".Build
scripts/build-bundle.jsesbuild gainspure: ['console.debug', 'console.info']. Withminify: truealready on, those calls get tree-shaken from production output (verified 0 occurrences indist/bundled/*.bundle.js).console.warn/console.errorpreserved on purpose so real degradation/errors still reach DevTools.Tests
tests/protected-terms.test.js(+6 tests): null/undefined/non-string safety, idempotence (f(f(x)) == f(x)), empty-wrong-form skip, self-mapping skip, non-string-array-element skip. Total 24 tests against the helper.Deferred
tsconfig.json+_sb-typedef.jsare the foundation; per-file JSDoc tightening is incremental work for follow-up PRs.docs/E2E_PLAN.mdcaptures the spec; needs a dedicated PR with browser fixtures + network stubs.chat-flashcards.jsextraction — flashcard logic is the most tangled withsavedChatHTMLpanel state; safer to split in a focused PR after the_sb._chatpanel API has been validated in production.fs.readFileSync+new Function, which istanbul cannot instrument. Real coverage tracking needs a test-harness refactor first.Test plan
npm test— 343/343 passnpm run lintcleannpm run format:checkcleannpm run validate/glossary/check:sync/check:dicts/check:i18n/check:selectorsall passnpm run build:bundle— succeeds, 0console.debug/console.infoin bundled outputnpm run build:firefox— succeedsdist/bundled, switch language to Korean on a Skilljar page, open sidebar, send chat, open history, open flashcards, close sidebar (verifies the_sb._chat.stateshared flags work end-to-end)