Add official chart explorer standard#10
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches🧪 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 introduces a new interactive chart explorer, enabling users to visualize financial time series data from the API. It includes a new /charts endpoint, a dedicated documentation file for chart standards, and a JavaScript implementation using the TradingView Lightweight Charts library. A review comment identifies an improvement opportunity in the parseTime function to support numeric Unix timestamps, which would increase the explorer's compatibility with different data sources.
| const parseTime = (value) => { | ||
| if (typeof value !== "string" && typeof value !== "number") return null; | ||
| const text = String(value).trim(); | ||
| let match = text.match(/^(\d{2})\/(\d{2})\/(\d{4})$/); | ||
| if (match) return `${match[3]}-${match[2]}-${match[1]}`; | ||
|
|
||
| match = text.match(/^(\d{4})(\d{2})$/); | ||
| if (match) return `${match[1]}-${match[2]}-01`; | ||
|
|
||
| match = text.match(/^(\d{4})-(\d{2})-(\d{2})/); | ||
| if (match) return `${match[1]}-${match[2]}-${match[3]}`; | ||
|
|
||
| const parsed = new Date(text); | ||
| if (Number.isNaN(parsed.getTime())) return null; | ||
| return parsed.toISOString().slice(0, 10); | ||
| }; |
There was a problem hiding this comment.
The parseTime function does not correctly handle numeric timestamps (Unix timestamps). When converting a numeric value to a string and passing it to new Date(), JavaScript fails to interpret purely numeric strings as dates (e.g., new Date("1714900000000") returns Invalid Date). Since many financial APIs return dates as timestamps, it is recommended to handle the number type explicitly to improve the explorer's compatibility with various data sources.
const parseTime = (value) => {
if (typeof value === "number") {
// Handle Unix timestamps (seconds vs milliseconds)
const date = new Date(value > 1e11 ? value : value * 1000);
return Number.isNaN(date.getTime()) ? null : date.toISOString().slice(0, 10);
}
if (typeof value !== "string") return null;
const text = value.trim();
let match = text.match(/^(\d{2})\/(\d{2})\/(\d{4})$/);
if (match) return `${match[3]}-${match[2]}-${match[1]}`;
match = text.match(/^(\d{4})(\d{2})$/);
if (match) return `${match[1]}-${match[2]}-01`;
match = text.match(/^(\d{4})-(\d{2})-(\d{2})/);
if (match) return `${match[1]}-${match[2]}-${match[3]}`;
const parsed = new Date(text);
if (Number.isNaN(parsed.getTime())) return null;
return parsed.toISOString().slice(0, 10);
};There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 117aba9312
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match = text.match(/^(\d{4})-(\d{2})-(\d{2})/); | ||
| if (match) return `${match[1]}-${match[2]}-${match[3]}`; |
There was a problem hiding this comment.
Preserve intraday timestamps when normalizing date fields
The ISO-date branch truncates any YYYY-MM-DDTHH:mm... value to YYYY-MM-DD, so intraday series are reduced to one point per day when deduped.set(time, ...) runs later. This corrupts chart data for supported endpoints that emit datetime candles (for example /yahoo/chart with intraday intervals), because multiple records collapse under the same key and only the last one survives.
Useful? React with 👍 / 👎.
| const dateKey = firstKey(firstRecord, DATE_KEYS); | ||
| if (!dateKey) throw new Error("Não encontrei campo de data conhecido."); | ||
|
|
||
| const shouldUseCandles = options.type === "candlestick" || hasOhlc(firstRecord); |
There was a problem hiding this comment.
Respect requested line mode for OHLC records
This condition forces candlestick rendering whenever the payload contains open/high/low/close, even if the preset explicitly asks for a scalar field (e.g., field: "close" in the Ibovespa preset). As a result, close-only comparisons cannot be rendered as lines for OHLC endpoints, which changes the intended visualization and summary semantics without user intent.
Useful? React with 👍 / 👎.
Summary
/chartsbrowser chart explorer using TradingView Lightweight Charts with the project brand color theme.docs/CHART_STANDARDS.mdand links it from the README.Root cause / fixes
/chartsroute.ultimosendpoint is capped for largen, so the default preset now uses a bounded date-range endpoint; andIntl.DateTimeFormatwith mixed style/time-zone options failed in the in-app browser, so extraction timestamps now use explicit BRT formatting.Validation
node --check src/findata/web/static/chart-explorer.js.venv/bin/ruff format --check src/ tests/ scripts/.venv/bin/ruff check src/ tests/ scripts/.venv/bin/python -m mypy src/findata.venv/bin/python -m pytest tests/ -qgit diff --checkhttp://127.0.0.1:8765/charts: initial series loaded asSérie plotada., 731 points, no console errors, repo source link and Lightweight Charts link present.UI evidence
/Users/macbook/Documents/Codex/2026-05-05/findata_charts_deploy_check.pngFontes dos dados... Dados Abertos de Mercado (findata-br)... Subsets originais: BCB SGS 432.