refactor(bible): centralize book-name maps in bible.ts#3
refactor(bible): centralize book-name maps in bible.ts#3euxaristia wants to merge 1 commit intomasterfrom
Conversation
Export LATIN_BOOK_NAMES, SPANISH_BOOK_NAMES, ABBREVIATION_MAP, and
default-version constants from bible.ts and import them in
message-handler.ts. Removes duplicated maps that had already drifted
("tobias" previously mapped to itself instead of "tobit", "rut" was
missing from bible.ts).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request refactors the codebase by exporting constants from src/services/bible.ts for use in src/services/message-handler.ts and adds support for the book of Ruth in the abbreviation map. The review identified a critical bug where Spanish references incorrectly default to the Latin Vulgate version, as well as a need to consolidate duplicate normalizeBookName logic. Additionally, the reviewer noted missing Spanish book names in the detection set and missing abbreviations for 'amós' and 'baruc' in the mapping.
| BibleService, | ||
| BibleVerse, | ||
| DEFAULT_LATIN_VERSION, | ||
| DEFAULT_SPANISH_VERSION, |
There was a problem hiding this comment.
In the processMessage function (line 691), fallbackVersion is incorrectly set to DEFAULT_LATIN_VERSION when a Spanish book name is detected. This causes all Spanish references (e.g., "Juan 3:16") to return Latin text from the Vulgate instead of the Spanish translation. It should be changed to DEFAULT_SPANISH_VERSION.
Additionally, the normalizeBookName function in this file (line 35) is now a duplicate of the one in bible.ts and contains a bug in its candidate sorting logic (it doesn't correctly find the minimum length across all abbreviations). Consider exporting the version from bible.ts and using it here to ensure consistency and fix the bug.
| export const SPANISH_BOOK_NAMES = new Set([ | ||
| "génesis", "éxodo", "exodo", "levítico", "levitico", "números", "numeros", | ||
| "deuteronomio", "josué", "josue", "jueces", "1 reyes", "2 reyes", | ||
| "1 crónicas", "1 cronicas", "2 crónicas", "2 cronicas", "esdras", |
There was a problem hiding this comment.
The SPANISH_BOOK_NAMES set is missing several books that should trigger Spanish auto-detection, specifically "rut" and "ester". While "rut" was added to the ABBREVIATION_MAP in this PR, it was missed here. Including these ensures that references like "Rut 1:1" or "Ester 2:5" are correctly identified as Spanish and use the appropriate default version.
| export const SPANISH_BOOK_NAMES = new Set([ | |
| "génesis", "éxodo", "exodo", "levítico", "levitico", "números", "numeros", | |
| "deuteronomio", "josué", "josue", "jueces", "1 reyes", "2 reyes", | |
| "1 crónicas", "1 cronicas", "2 crónicas", "2 cronicas", "esdras", | |
| export const SPANISH_BOOK_NAMES = new Set([ | |
| "génesis", "éxodo", "exodo", "levítico", "levitico", "números", "numeros", | |
| "deuteronomio", "rut", "ester", "josué", "josue", "jueces", "1 reyes", "2 reyes", | |
| "1 crónicas", "1 cronicas", "2 crónicas", "2 cronicas", "esdras", |
| "josué": "joshua", | ||
| "josue": "joshua", | ||
| "jueces": "judges", | ||
| "rut": "ruth", |
Summary
LATIN_BOOK_NAMES,SPANISH_BOOK_NAMES,ABBREVIATION_MAP,DEFAULT_LATIN_VERSION,DEFAULT_SPANISH_VERSIONfrombible.tsand imports them inmessage-handler.ts, eliminating ~370 lines of duplicated constants."tobias"was self-mapped in message-handler (should be"tobit")."rut"(Spanish) was missing from bible.ts's map.Test plan
deno test -A tests/bible.test.ts tests/message-handler.test.ts— 130/130 passing