feat: wire zakat into chat, live nisab pricing, and test coverage (#22) - #68
Merged
Merged
Conversation
…erage Zakat v1 shipped a working /zakat endpoint that the assistant itself could not use, with a nisab frozen in an env var and no automated tests. Live nisab (nisab.py) The nisab is the value of 85g of gold, so it moves with the gold market and a hardcoded figure drifts wrong in both directions. It is now derived from a spot price via gold-api.com, falling back to CoinGecko's PAX Gold (redeemable one-for-one for a troy ounce of allocated gold) and then to ZAKAT_NISAB_USD. Neither source needs a key. Prices are cached for NISAB_CACHE_TTL_SECONDS, and a price outside a plausible range is refused rather than used, so a source that changes its units degrades instead of producing a nisab off by an order of magnitude. Every quote reports whether it is live and where it came from, so a fallback is never mistaken for a market figure. Chat integration A zakat question carrying a Stellar public key — in the prompt or in the request's context field — now reads the real on-chain balance and hands the model the actual figures, the nisab basis, and an instruction to close with the existing scholar disclaimer verbatim. Without a key the assistant explains the calculation and invites a public key. Detection is offline (keywords plus a key-shaped match), so an ordinary message reaches neither Horizon nor the price API, and a zakat answer is never written to the response cache: it contains one user's real balance. Read-only, and harder to misuse Secret keys already failed validation. Two gaps are now closed: a message that looks like it contains one is refused with a warning rather than used, and the key is redacted from the prompt before it reaches the model provider or the stored history — the module promises secret keys are never stored or logged, and that has to survive a user volunteering one. Also: the synchronous Horizon call now runs off the event loop rather than blocking every other in-flight request, and zakat arithmetic is quantized to USDC's 7 decimal places. Tests (tests/test_zakat.py, 79 offline) Zakat math above/below/exactly at nisab, that 2.5% applies to the whole balance rather than the excess, quantization, key validation including secret-key rejection, Horizon mocked for the trustline / no-trustline / wrong-issuer / unknown-account paths, the response shape, live nisab with both sources and every fallback, cache behaviour, chat detection, and redaction. No live network calls. Closes Deen-Bridge#22
WalkthroughAdds live gold-price nisab calculation with fallback and caching, refactors Stellar zakat computation into reusable helpers, integrates zakat context into chat with secret-key redaction, and adds extensive offline tests and CI coverage. ChangesZakat v2
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
stellar.py (1)
219-244: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMinor false-positive in keyword detection:
"alms"matches"psalms".
is_zakat_questiondoes a plain substring scan, so a prompt like "explain the psalms" trips the zakat path (because"alms"⊂"psalms"). It's low-harm — the worst case is an unsolicitedNO_KEY_NOTEexplanation — but if you want to tighten it, a word-boundary regex over the keyword set avoids the surprise. Entirely optional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stellar.py` around lines 219 - 244, Update is_zakat_question to match ZAKAT_KEYWORDS as whole words rather than using substring checks, preventing “alms” from matching words such as “psalms” while preserving detection of standalone keywords. Reuse the existing keyword set and keep the current empty-text handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@stellar.py`:
- Around line 219-244: Update is_zakat_question to match ZAKAT_KEYWORDS as whole
words rather than using substring checks, preventing “alms” from matching words
such as “psalms” while preserving detection of standalone keywords. Reuse the
existing keyword set and keep the current empty-text handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8aae2354-8e26-43e4-8ea5-af4607eac63f
📒 Files selected for processing (8)
.env.example.github/workflows/ci.ymlREADME.mdmain.pynisab.pysemantic_cache.pystellar.pytests/test_zakat.py
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
2 participants
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.
Closes #22
Zakat v1 shipped a working
/zakatendpoint the assistant itself couldn't use, a nisab frozen in an env var, and no tests. This closes all three.Live nisab —
nisab.pyThe nisab is the value of 85g of gold, so it moves with the market: a hardcoded figure drifts wrong in both directions, telling some users they owe nothing when they do and vice versa. It is now derived from a spot price:
ZAKAT_NISAB_USD— the configured default, when neither is reachableNeither API needs a key. Prices are cached for
NISAB_CACHE_TTL_SECONDS(6h default) through the existingsemantic_cache.KeyedCache— I added an optional per-entry TTL rather than a new cache, since a market price is worth caching for hours, not for the day-long TTL that suits an immutable tafsir passage.A price outside a plausible range ($100–$100,000/oz) is refused rather than used: if a source changes its units or field meaning, the right outcome is degrading to the next source, not silently producing a nisab off by an order of magnitude. Every quote reports
live: true/falseand its source, so a fallback is never mistaken for a market figure. The 85g-gold basis is stated explicitly in the response — silver gives a substantially different threshold, and which applies is a matter scholars differ on.Chat integration
A zakat question carrying a Stellar public key — in the prompt, or in the request's
contextfield as the frontend passes it — now reads the real on-chain balance and hands the model the actual figures plus an instruction to close with the existing scholar disclaimer verbatim. Without a key, the assistant explains how zakat is calculated and invites a public key.Detection is offline (keywords plus a key-shaped match), so an ordinary message reaches neither Horizon nor the price API — there's a test asserting exactly that. A zakat answer is also never written to the response cache: it contains one user's real balance and must never be replayed to anyone else.
ChatResponsegains an additive optionalzakatblock.Verified end-to-end against a live gold price with a mocked balance: a 50,000 USDC wallet against a $4,063.70/oz gold price gives a nisab of $11,105.33 and 1,250 USDC due, and the model prompt carries the real figures rather than an example.
Read-only, and harder to misuse
Secret keys already failed validation, so one could never reach Horizon. Two gaps are now closed:
Also fixed while in here
stellar_sdk.Servercall ran directly inside an async endpoint, blocking the event loop for every other in-flight request. It now runs viaasyncio.to_thread, concurrently with the nisab fetch.Tests —
tests/test_zakat.py, 79 offlineZakat math above / below / exactly at the nisab; that 2.5% applies to the whole balance rather than the excess (a common misreading, asserted explicitly); quantization and large-balance exactness; key validation including bad checksums and secret-key rejection; Horizon mocked for the trustline / no-trustline / wrong-issuer / unknown-account paths; the recorded response shape; live nisab across both sources and every fallback path including an implausible price; cache behaviour, including that a fallback is not cached so an outage can't pin the default in place for hours; chat detection; and redaction.
No live network calls — Horizon is mocked and the price API is stubbed with
httpx.MockTransport. CI extends flake8/compile tonisab.pyand adds the new pytest step. Full suite: 541 passed, 1 skipped.Note
README.mddocuments the price sources, the fallback chain, and the chat behaviour. This branch also touchesmain.pyand.env.example, which #67 (docs) touches too — whichever merges second, I'll resolve.Summary by CodeRabbit
New Features
Bug Fixes
Tests