refactor: dedup B3 quote guard and push anbima emissor into the source#31
Conversation
New api/_b3_common.py exposes resolve_quotes() and MAX_TICKERS; routers/b3.py and mcp_app.py both resolve the lazy yfinance import through it, so the guard and the install hint live in one place instead of two near-copies. anbima.get_debentures now takes an emissor filter; the REST router and the MCP tool drop their hand-rolled loop and pass it through, matching every other source-level filter. Items 2 and 3 of #30. No behavior change; ruff/mypy/pytest (252) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughB3 quote resolution was centralized into a shared helper used by the MCP app and B3 router handlers, with a shared max-ticker limit. ANBIMA debenture filtering now happens in ChangesB3 quote resolver centralization
ANBIMA debenture emissor filtering
Sequence Diagram(s)sequenceDiagram
participant b3_quote
participant resolve_quotes
participant quotes_module as findata.sources.b3.quotes
b3_quote->>resolve_quotes: resolve quotes backend
resolve_quotes->>quotes_module: lazy import
quotes_module-->>b3_quote: get_quote / get_history / get_multiple_quotes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request refactors the B3 live-quote logic by consolidating shared helpers into a new _b3_common.py module, and optimizes debenture querying by pushing the emissor filtering logic down to the source layer in get_debentures. A critical issue was identified in resolve_quotes() where the lazy import of quotes will not raise an ImportError when yfinance is missing, potentially causing unexpected 500 or 404 errors instead of the intended 503; a suggestion was provided to explicitly import yfinance in the try block.
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.
| def resolve_quotes() -> Any: | ||
| """Return the yfinance-backed quotes module, or raise 503 if the extra is missing.""" | ||
| try: | ||
| from findata.sources.b3 import quotes | ||
| except ImportError as exc: # pragma: no cover - only without the [b3] extra | ||
| raise HTTPException( | ||
| status_code=503, | ||
| detail="B3 live quotes need the optional extra: pip install 'openfindata[b3]'", | ||
| ) from exc | ||
| return quotes |
There was a problem hiding this comment.
The quotes module (src/findata/sources/b3/quotes.py) imports yfinance lazily inside its functions rather than at the module level. As a result, from findata.sources.b3 import quotes will not raise an ImportError when yfinance is missing. This means resolve_quotes() will successfully return the module, and the missing dependency will only trigger a RuntimeError later when a quote is actually fetched. This leads to a 500 Internal Server Error in the MCP app or a misleading 404 Ticker not found in the REST router instead of the intended 503 Service Unavailable with the installation hint.
To fix this, explicitly attempt to import yfinance inside the try block to correctly detect if the optional extra is missing.
| def resolve_quotes() -> Any: | |
| """Return the yfinance-backed quotes module, or raise 503 if the extra is missing.""" | |
| try: | |
| from findata.sources.b3 import quotes | |
| except ImportError as exc: # pragma: no cover - only without the [b3] extra | |
| raise HTTPException( | |
| status_code=503, | |
| detail="B3 live quotes need the optional extra: pip install 'openfindata[b3]'", | |
| ) from exc | |
| return quotes | |
| def resolve_quotes() -> Any: | |
| """Return the yfinance-backed quotes module, or raise 503 if the extra is missing.""" | |
| try: | |
| import yfinance # noqa: F401 | |
| from findata.sources.b3 import quotes | |
| except ImportError as exc: # pragma: no cover - only without the [b3] extra | |
| raise HTTPException( | |
| status_code=503, | |
| detail="B3 live quotes need the optional extra: pip install 'openfindata[b3]'", | |
| ) from exc | |
| return quotes |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/findata/api/_b3_common.py`:
- Around line 17-25: The missing-[b3] path is no longer caught by the
ImportError in resolve_quotes(), because importing findata.sources.b3.quotes now
succeeds before yfinance is needed. Update resolve_quotes() to verify the
optional dependency at import time by triggering the yfinance-backed path or
calling the helper that loads it from quotes, and keep raising the shared
HTTPException 503 with the install hint when the extra is absent. Use the
existing resolve_quotes() helper and the lazy import flow in
findata.sources.b3.quotes to make sure callers always get the intended 503
instead of a later RuntimeError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa37420e-b1a6-4088-90b3-bab63be4964e
📒 Files selected for processing (5)
src/findata/api/_b3_common.pysrc/findata/api/mcp_app.pysrc/findata/api/routers/anbima.pysrc/findata/api/routers/b3.pysrc/findata/sources/anbima/indices.py
| def resolve_quotes() -> Any: | ||
| """Return the yfinance-backed quotes module, or raise 503 if the extra is missing.""" | ||
| try: | ||
| from findata.sources.b3 import quotes | ||
| except ImportError as exc: # pragma: no cover - only without the [b3] extra | ||
| raise HTTPException( | ||
| status_code=503, | ||
| detail="B3 live quotes need the optional extra: pip install 'openfindata[b3]'", | ||
| ) from exc |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The missing-[b3] path no longer turns into the shared 503 response.
findata.sources.b3.quotes now defers the yfinance import to _import_yfinance() in src/findata/sources/b3/quotes.py:70-77, so from findata.sources.b3 import quotes succeeds even when the extra is absent. In that case this helper returns normally, and the later quote call raises RuntimeError, which the REST/MCP callers currently translate into 404/500 instead of the advertised 503 install hint.
Possible fix
+from importlib.util import find_spec
from typing import Any
from fastapi import HTTPException
@@
def resolve_quotes() -> Any:
"""Return the yfinance-backed quotes module, or raise 503 if the extra is missing."""
+ if find_spec("yfinance") is None:
+ raise HTTPException(
+ status_code=503,
+ detail="B3 live quotes need the optional extra: pip install 'openfindata[b3]'",
+ )
try:
from findata.sources.b3 import quotes📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def resolve_quotes() -> Any: | |
| """Return the yfinance-backed quotes module, or raise 503 if the extra is missing.""" | |
| try: | |
| from findata.sources.b3 import quotes | |
| except ImportError as exc: # pragma: no cover - only without the [b3] extra | |
| raise HTTPException( | |
| status_code=503, | |
| detail="B3 live quotes need the optional extra: pip install 'openfindata[b3]'", | |
| ) from exc | |
| from importlib.util import find_spec | |
| from typing import Any | |
| from fastapi import HTTPException | |
| def resolve_quotes() -> Any: | |
| """Return the yfinance-backed quotes module, or raise 503 if the extra is missing.""" | |
| if find_spec("yfinance") is None: | |
| raise HTTPException( | |
| status_code=503, | |
| detail="B3 live quotes need the optional extra: pip install 'openfindata[b3]'", | |
| ) | |
| try: | |
| from findata.sources.b3 import quotes | |
| except ImportError as exc: # pragma: no cover - only without the [b3] extra | |
| raise HTTPException( | |
| status_code=503, | |
| detail="B3 live quotes need the optional extra: pip install 'openfindata[b3]'", | |
| ) from exc |
🤖 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 `@src/findata/api/_b3_common.py` around lines 17 - 25, The missing-[b3] path is
no longer caught by the ImportError in resolve_quotes(), because importing
findata.sources.b3.quotes now succeeds before yfinance is needed. Update
resolve_quotes() to verify the optional dependency at import time by triggering
the yfinance-backed path or calling the helper that loads it from quotes, and
keep raising the shared HTTPException 503 with the install hint when the extra
is absent. Use the existing resolve_quotes() helper and the lazy import flow in
findata.sources.b3.quotes to make sure callers always get the intended 503
instead of a later RuntimeError.
What
Two contained cleanups from the review of #29 (items 2 and 3 of #30):
api/_b3_common.pyexposesresolve_quotes()andMAX_TICKERS.routers/b3.pyandmcp_app.pyboth resolve the lazyyfinanceimport through it, so the guard and thepip install 'openfindata[b3]'hint live in one place instead of two near-copies.emissorfilter into the source.anbima.get_debentures(...)now takesemissor, and both the REST router and the MCP tool drop their hand-rolled filter loop and pass it through, matching every other source-level filter (FCA ticker, IPE categoria, FIP quarter).Net
-23lines, no behaviour change.ruff format/ruff check/mypy --strict/pytest -m "not integration"(252) green.Item 1 of #30 deliberately left out
The third item (push
limitinto ~18 source functions) is not included here. The unbounded-output bug that motivated it was already fixed in #29 (the MCP tools apply[:limit]). What remains is purely moving the slice into the source functions, which (a) gives no perf win (the CSV/zip is parsed in full regardless), (b) does not unify the REST routers, which needskip+limitvia_page, and (c) adds alimitparam to 18 public source functions for marginal DRY. Recommend closing #30 with items 2-3, or splitting item 1 into its own ticket if still wanted.Summary by CodeRabbit
New Features
Bug Fixes