Migrate scrabble module to Conductor#792
Conversation
scrabble exports four static word/letter lists rather than functions, so there's nothing for the usual exportedNames/@moduleMethod closure path to wrap. Exposes them via DataType.OPAQUE (opaque_make in an initialise() override) instead of DataType.ARRAY, since array_make has no bulk constructor and the full word list is 172,820 entries. Also drops the two full-array snapshot tests (scrabble_words/ scrabble_letters) - snapshotting 172,820 entries produced a multi-million-line .snap file that hung vitest's serializer. The existing index spot-checks already cover the full arrays; only the ~1,728-entry _tiny variants are snapshotted now. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request refactors the scrabble bundle to export a ScrabbleModulePlugin extending BaseModulePlugin instead of directly exporting the word lists. The word lists are now exposed as opaque values asynchronously during initialization. Additionally, snapshot tests for the full-sized word lists were removed to prevent test runner hangs, and a new test was added to verify the plugin's initialization behavior. Feedback is provided to optimize the initialization performance by executing the asynchronous opaque_make calls concurrently using Promise.all instead of sequentially.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
The four exports are independent, so awaiting them one at a time added needless latency. Addresses gemini-code-assist review comment on source-academy#792. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolves conflicts in scrabble's package.json/yarn.lock (typescript version now tracks the workspace catalog instead of a local pin, matching upstream's cleanup). Also adds the `override` modifier to `exportedNames`/`channelAttach` in index.ts - conductor-migration picked up `noImplicitOverride` since this branch diverged, same fix already applied to `repeat`'s index.ts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
array_make's lack of a bulk constructor (one array_set call per element) looked prohibitive for 172,820 words, so this used opaque_make instead. That was never measured, and the actual cost is ~246ms one-time (TestDataHandler, same-thread as the evaluator - no postMessage boundary between a module and its evaluator) for both scrabble_words and the nested scrabble_letters. Cheap enough that there's no reason to give up real indexing/print/iteration for it. Follows from source-academy/py-slang#217's module-interop fixes and the team's decision that Python lists/JS arrays should be the only built-in data structure modules hand back - opaque_make stays reserved for genuinely opaque payloads (e.g. binary_tree's node values), not plain collections. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Description
Fixes #782
Migrates the
scrabblemodule to be Conductor-ready.scrabblehas no tab/visualization component, so this is scoped entirely tosrc/bundles/scrabble/.Unlike
repeat/binary_tree/midi, scrabble exports four static word/letter lists, not functions - there's nothing for the usualexportedNames/@moduleMethodclosure path to wrap.index.tsis now aBaseModulePluginsubclass that pushes all four exports in aninitialise()override, built as realDataType.ARRAYs viaarray_make/array_set.DataType.OPAQUEwas the initial choice here (see review history) -IDataHandler.array_makehas no bulk constructor, so populating an array is onearray_setcall per element, andscrabble_wordsalone is 172,820 entries. That looked prohibitive on paper. It wasn't measured, though, and the actual cost (via@sourceacademy/modules-testplugin'sTestDataHandler) is ~246ms one-time for bothscrabble_wordsand the nestedscrabble_letterscombined - negligible at module-load time, and a module's evaluator runs in the same execution context (the Runner), so there's nopostMessageboundary inflating that further in production.opaque_makestays the right call for genuinely opaque payloads (e.g.binary_tree's node values,rune's whole Rune object) - the difference is scrabble's data isn't opaque at all, it's a plain word list, so wrapping it that way just cost studentsprint/indexing/iteration for no real reason.This also follows source-academy/py-slang#217's module-interop fixes and the resulting team decision that Python lists / JS arrays should be the only built-in data structure modules hand back, rather than reaching for
opaque_makeon anything compound.functions.tsis untouched - it's pure data derivation (map/filter over the word list), nothing conductor-specific about it.Unrelated bug fixed along the way
The existing
scrabble_words matches snapshot/scrabble_letters matches snapshottests hung vitest indefinitely - confirmed via bisection againstrepeat's suite (clean, 1.2s) and this module's new test in isolation via.only(clean, 2.76s). Snapshotting a 172,820-entry array (and the nestedscrabble_letters) produced a 2.1M-line.snapfile that vitest's serializer choked on. Dropped those two tests and deleted the stale snapshot file; the existing index spot-checks (scrabble_words[12],scrabble_letters[100000]) already cover the full arrays cheaply, and the_tinyvariants (~1,728 entries) still get proper snapshots (regenerated, 2 written).Testing
yarn workspace @sourceacademy/bundle-scrabble run tsc- passesyarn workspace @sourceacademy/bundle-scrabble run lint- passesyarn workspace @sourceacademy/bundle-scrabble run test- 5/5 passing, against@sourceacademy/modules-testplugin'sTestDataHandleryarn workspace @sourceacademy/bundle-scrabble run build- producesbuild/bundles/scrabble.jsChecklist