-
Notifications
You must be signed in to change notification settings - Fork 17
Fix Genesis 17 verse count to 27 #224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Genesis 17 verse count to 27 #224
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the Genesis 17 verse count in the core verse metadata and adds a regression test to validate the corrected count via the public counting API. Sequence diagram for verse count lookup using the public counting APIsequenceDiagram
participant Test as counters_test
participant API as counting_api
participant Data as verse_metadata
Test->>API: get_number_of_verses(book=GENESIS, chapter=17)
API->>Data: lookup_verse_count(book=GENESIS, chapter=17)
Data-->>API: verse_count=27
API-->>Test: 27
Note over Test,API: Regression test confirms Genesis 17 now returns 27 verses
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
The scope of this PR may be expanded depending on continued discussion in #223. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 1 issue, and left some high level feedback:
- The PR title/description and linked issue say Genesis 17 should have 28 verses, but the code and test enforce 27; please verify the correct verse count and align the metadata, test assertion, and PR description accordingly.
- Instead of using
# type: ignore[arg-type]intest_count_verses_genesis_17, consider constructing the expected reference type (e.g., aReferenceobject) so the call tobible.count_versesis type-safe without suppressing type checking.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The PR title/description and linked issue say Genesis 17 should have 28 verses, but the code and test enforce 27; please verify the correct verse count and align the metadata, test assertion, and PR description accordingly.
- Instead of using `# type: ignore[arg-type]` in `test_count_verses_genesis_17`, consider constructing the expected reference type (e.g., a `Reference` object) so the call to `bible.count_verses` is type-safe without suppressing type checking.
## Individual Comments
### Comment 1
<location> `tests/counters_test.py:215-222` </location>
<code_context>
assert number_of_verses == 1 + 1 + (3 + 1)
+
+
+def test_count_verses_genesis_17() -> None:
+ # To address https://github.com/avendesora/pythonbible/issues/223
+ reference: str = "Genesis 17"
+
+ # Retrieve number of verses in Genesis 17
+ number_of_verses: int = bible.count_verses(reference) # type: ignore[arg-type]
+
+ # Genesis 17 should have 27 verses
+ assert number_of_verses == 27
</code_context>
<issue_to_address>
**issue (testing):** Test expectation (27 verses) appears to contradict the PR title/description and linked issue (27 vs 28).
This test and its comment assume Genesis 17 has 27 verses, while the PR title/description and linked issue describe a change “to 27 from 28,” and the code updates metadata from 28 to 27. Please confirm the correct verse count from an authoritative source and align the test, metadata, and PR/issue text accordingly so we don’t bake an incorrect expectation into the tests or documentation.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Description
Minimal bug fix updating the verse count for Genesis 17 from 28 to 27, with a confirming test. Addresses #223.
Checklist:
Summary by Sourcery
Correct the stored verse count for Genesis 17 and add regression coverage to ensure the count remains accurate.
Bug Fixes:
Tests: