Skip to content

Claude/analyze us exchange g6 b kg#8

Open
axelqap wants to merge 3 commits into
banbox:mainfrom
axelqap:claude/analyze-us-exchange-g6BKg
Open

Claude/analyze us exchange g6 b kg#8
axelqap wants to merge 3 commits into
banbox:mainfrom
axelqap:claude/analyze-us-exchange-g6BKg

Conversation

@axelqap

@axelqap axelqap commented May 10, 2026

Copy link
Copy Markdown

An assessment of how yahoo works to download data from the USA using OHLCV.

Summary by CodeRabbit

  • New Features
    • Added Yahoo Finance as a supported exchange provider for accessing historical and real-time market data
    • Retrieve OHLCV candlesticks across multiple timeframes including 1-minute, hourly, and daily intervals
    • Fetch ticker prices and quote information for multiple symbols

Review Change Stack

claude added 2 commits May 10, 2026 15:14
Adds a new `yahoo` exchange providing free US-market data (OHLCV +
tickers) via Yahoo Finance's public v8 chart and v7 quote endpoints.
Hand-rolled (no extra dependency) so requests use banexg's existing
HTTP plumbing — proxy, retries, caching, debug, rate limit — instead
of the global state used by piquette/finance-go.

Scope: read-only. FetchOHLCV (1m/5m/15m/30m/1h/1d/1wk/1mo etc.),
FetchTicker(s)/FetchTickerPrice, lazy LoadMarkets/MapMarket. Trading
methods inherit base NotImplement stubs.
Adds the missing 4h timeframe by fetching 1h from Yahoo and bucketing
into 4h windows aligned to UTC. Also accepts the uppercase aliases
1H/1D/1W/1M that users commonly pass through from UI layers.

Adds an offline aggregator unit test and a parameterised AAPL live
test (TestFetchOHLCV_AAPL_AllTimeframes) covering 1m/5m/1h/4h/1D/1W/1M;
gated by BANEXG_YAHOO_LIVE=1 so it skips in offline CI.
@coderabbitai

coderabbitai Bot commented May 10, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@axelqap has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 47 minutes and 24 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 73d44c26-4019-4844-8c4c-c747cee75b5c

📥 Commits

Reviewing files that changed from the base of the PR and between 1b2b975 and 63df831.

📒 Files selected for processing (5)
  • yahoo/biz.go
  • yahoo/biz_test.go
  • yahoo/common.go
  • yahoo/common_test.go
  • yahoo/entry.go
📝 Walkthrough

Walkthrough

This PR adds a complete Yahoo Finance exchange client to banexg, enabling OHLCV candle and ticker quote fetching. The implementation includes lazy market loading, timeframe validation, JSON parsing, 4-hour interval aggregation, and parametrized range selection, with comprehensive unit and integration tests.

Changes

Yahoo Finance Exchange Client

Layer / File(s) Summary
Exchange Registration
bex/entrys.go
Yahoo exchange is imported and registered in the newExgs factory map.
Data Types and Configuration
yahoo/types.go, yahoo/data.go
Exchange wrapper type, API response structs for chart and quote endpoints, error models, and endpoint constants are defined.
Parsing and Utility Functions
yahoo/common.go
Request signing, ticker splitting, chart/quote JSON parsing, interval-to-range mapping, and kline aggregation for custom timeframes are implemented.
Factory and Initialization
yahoo/entry.go
New() creates Yahoo exchange instances with configuration, metadata, rate limits, and request signing; NewExchange() wraps for interface compliance.
Exchange API Implementation
yahoo/biz.go
Init() sets up timeframe mappings; LoadMarkets() and MapMarket() handle lazy market registration; FetchOHLCV() retrieves candles with retry and special 4-hour aggregation; FetchTickers(), FetchTicker(), and FetchTickerPrice() retrieve quotes; Close() is a no-op.
Unit Tests for Utilities
yahoo/common_test.go
Tests for splitTicker, parseChart, parseQuote, aggregate, and chooseRange validate parsing logic and range selection.
Integration Tests for APIs
yahoo/biz_test.go
Offline construction, market mapping, and live OHLCV/ticker fetching tests across multiple timeframes with bar count/OHLC sanity checks; tests are gated by BANEXG_YAHOO_LIVE environment variable.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A new exchange hops into the fold,
Yahoo's markets sparkle bright with gold.
Charts aggregate by hour into days,
Four-hour candles light the trading ways.
Ticker quotes fetch swift and true,
The banexg warren grows anew!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Claude/analyze us exchange g6 b kg' is vague and cryptic. It contains branch naming artifacts and does not clearly communicate the main change: adding Yahoo Finance exchange support. Revise the title to clearly describe the main change, such as 'Add Yahoo Finance exchange implementation for US market OHLCV data' or 'Implement Yahoo Finance exchange client for market data retrieval'.
Docstring Coverage ⚠️ Warning Docstring coverage is 29.63% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@yahoo/biz.go`:
- Around line 64-66: The current direct assertion v.([]string) for
params[banexg.ParamSymbols] is too strict and drops valid inputs; update the
decoding in both places where you read params[banexg.ParamSymbols] to handle
three cases: if v is []string use it directly, if v is []interface{} iterate and
convert each element to string (using fmt.Sprint or type switches) to build
symbols []string, and if v is a string treat it as a comma-separated list and
split into []string (trimming whitespace). Ensure the result is assigned to the
existing symbols variable and handle nil/empty inputs gracefully.

In `@yahoo/entry.go`:
- Around line 9-14: The constructor New currently mutates the caller-provided
Options map; instead, create a local copy and mutate that: in New, allocate a
new map (e.g., opts := make(map[string]interface{}, len(Options)+1)), if Options
!= nil copy each key/value from Options into opts, otherwise start with an empty
map, then set the default value for banexg.OptUserAgent to defaultUserAgent on
opts if missing, and use opts for subsequent operations so the original Options
argument is not modified.
🪄 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: fa29b3c5-942e-44d8-9e37-3fdf016e1470

📥 Commits

Reviewing files that changed from the base of the PR and between 5c011e1 and 1b2b975.

📒 Files selected for processing (8)
  • bex/entrys.go
  • yahoo/biz.go
  • yahoo/biz_test.go
  • yahoo/common.go
  • yahoo/common_test.go
  • yahoo/data.go
  • yahoo/entry.go
  • yahoo/types.go

Comment thread yahoo/biz.go Outdated
Comment thread yahoo/entry.go Outdated
* coerceSymbols(): accept []string, []interface{}, and comma-separated
  strings for the ParamSymbols param. Direct v.([]string) was silently
  dropping JSON-decoded payloads and human-typed config strings.
  Used in LoadMarkets and FetchTickerPrice.

* New(): copy the caller's Options map via utils.SafeParams before
  injecting the default User-Agent, so callers sharing one Options map
  across multiple exchanges aren't affected.

Adds TestCoerceSymbols (9 subcases) and TestNew_DoesNotMutateOptions
as regression guards.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants