issue #15 [RFC] Include Istiaatha & Bismillah in Phonetic Search - #22
Open
amiraelgarf wants to merge 1 commit into
Open
issue #15 [RFC] Include Istiaatha & Bismillah in Phonetic Search#22amiraelgarf wants to merge 1 commit into
amiraelgarf wants to merge 1 commit into
Conversation
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.
Add Istiaatha & Bismillah support to
PhoneticSearchSummary
PhoneticSearchpreviously indexed only the core Quranic text (6,236 ayat), so queries containing الاستعاذة ("أعوذ بالله من الشيطان الرجيم") or البسملة ("بسم الله الرحمن الرحيم") ( both extremely common in real recitation audio ) could never match, since those phonemes simply didn't exist in the reference string.This PR adds both as first-class, filterable segments in the phoneme index, without breaking any existing query.
What changed
All changes are contained to
src/quran_transcript/phonetics/search.py.ph_index.npygains an 8th column,segment_type(0=quran, 1=istiaatha, 2=bismillah). Istiaatha is now indexed before every sura's first aya; Bismillah is indexed before every sura's first aya except sura 9 (not recited) and sura 1 (it's textually aya 1 itself, already indexed assegment_type=quran).PhoneticSearch.__init__(data_dir=None, start=(1,1), end=(114,6)): new optionalstart/endbounds to scope search to a(sura, aya)range. Rejects old 7-column indexes with a clearValueErrorinstead of silently misbehaving.search(..., include_istiaatha=False, include_bismillah=False): both default off (unchanged behavior for existing callers); matches touching an excluded segment are filtered out.get_uthmani_from_result(): now reconstructs Uthmani text correctly for matches inside Istiaatha, inside Bismillah, or spanning Istiaatha → Bismillah → Quran text in a single result. RaisesNotImplementedError(documented, not silent) for the one unhandled edge case: a match starting in one sura's Quran text and ending in the next sura's Istiaatha/Bismillah.Design notes
AyaFormat.bismillah_uthmani, which is alreadyNonefor those two suras inutils.py, avoiding a second source of truth.phonemes_idxwas always aya-local (reset per aya), not a global string offset: so prepending Istiaatha/Bismillah before each sura does not shift any existing aya's indices. Verified empirically (see Testing).Migration
Requires a one-time index rebuild:
Old (7-column)
ph_index.npyfiles are rejected at load time with a clear message rather than producing incorrect results.Testing
Verified against a real rebuild of the full Quran index (not just unit-level):
test_phonemes_search_pytest.py: 2/2 pass, unmodified, against the rebuilt 8-column index.test_istiaatha_bismillah_search.py(17 tests): index shape/row counts, inclusion/exclusion for both segment types, no false-positive Bismillah match in sura 9, correct handling of sura 1 and sura 27's literal Quranic Bismillah occurrences, boundary filtering, full Istiaatha+Bismillah+aya round-trip reconstruction (sura 112), theNotImplementedErroredge case, and old-index rejection.limitation
get_uthmani_from_result()doesn't yet resolve a match that starts in one sura's Quran text and ends in the following sura's Istiaatha/Bismillah, it raisesNotImplementedErrorrather than returning wrong text. Flagging as a follow-up rather than blocking this PR on it, since it's a narrow edge case at sura boundaries specifically.