Address post-merge chart review feedback

[robertoecf]

Summary

• Follow-up for the two Codex bot comments that arrived on PR Address chart lab review debt #13 after it had already been merged.
• Parses compact numeric period codes like 202401 and 20240131 before applying Unix timestamp fallback.
• Normalizes mixed string-date and Unix-timestamp records into a consistent timestamp representation before sorting.

Review feedback addressed

• PR Address chart lab review debt #13 Codex P1: parse numeric period codes before Unix timestamp fallback.
• PR Address chart lab review debt #13 Codex P2: order mixed time representations chronologically.

Validation

• node --check src/findata/web/static/chart-explorer.js
• .venv/bin/python -m pytest tests/test_api.py -q
• .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/ -q
• git diff --check

Merge note

Do not merge unless there are no open bot review comments/threads and Roberto explicitly approves merge in the current conversation.

Summary by CodeRabbit

• Refactor

  • Improved time parsing to recognize compact date strings, Unix timestamps (including shorter second-like values), and multiple ISO/date formats; parsing is more consistent across mixed inputs.
  • Enhanced data normalization and sorting to correctly handle mixed time representations and intraday (numeric) times.

• Tests

  • Added assertions to validate the enhanced time parsing and mixed-time normalization behavior.

[coderabbitai]

Warning

Rate limit exceeded

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

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 @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: f8da782f-802b-49dd-a153-4ea3c4a0751b

📥 Commits

Reviewing files that changed from the base of the PR and between faca4e0 and d90d99d.

📒 Files selected for processing (2)

• src/findata/web/static/chart-explorer.js
• tests/test_api.py

📝 Walkthrough

Walkthrough

Refactors time parsing and normalization in chart-explorer.js: adds isValidDateParts, parseCompactPeriod, parseUnixTimestamp, timeSortValue, and normalizeMixedTimes; updates parseTime and normalizeData to use the new helpers and return accurate hasIntraday information. Test assertions updated accordingly.

Changes

Time Parsing and Normalization Refactor

Layer / File(s)	Summary
New Helper Functions src/findata/web/static/chart-explorer.js	Adds isValidDateParts, parseCompactPeriod (converts YYYYMMDD/YYYYMM to ISO), and parseUnixTimestamp (interprets 10–13 digit timestamps, optional short-second handling) to centralize time parsing.
Parsing Logic Updates src/findata/web/static/chart-explorer.js	parseTime now routes inputs through parseCompactPeriod and parseUnixTimestamp (handles numeric inputs), then falls back to legacy DD/MM/YYYY, YYYY-MM-DD, YYYY-MM-DDT00:00:00, and new Date(text) parsing.
Mixed-Time Normalization src/findata/web/static/chart-explorer.js	Introduces timeSortValue and normalizeMixedTimes to detect numeric vs non-numeric point.time and convert intraday non-numeric times into numeric sort values; returns { data, hasIntraday }.
Data Normalization Wiring src/findata/web/static/chart-explorer.js	normalizeData uses normalizeMixedTimes result for hasIntraday and sorts deduped points using numeric subtraction when hasIntraday is true, otherwise localeCompare.
Tests / Verification tests/test_api.py	test_chart_explorer_asset updated to assert presence of isValidDateParts, parseCompactPeriod, parseUnixTimestamp (including allowShortSeconds usage), unixTimestamp !== null, normalizeMixedTimes, and the normalizedTime.hasIntraday ? a.time - b.time : a.time.localeCompare(b.time) sorting expression in /site/chart-explorer.js.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through dates both terse and long,
Turned strings and ticks into one clear song.
Compact months and seconds now align,
Mixed times sorted, tidy and fine.
Charts breathe easy — carrot cake for the mind. 🥕✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Address post-merge chart review feedback' is vague and generic, using non-descriptive language that doesn't convey the specific technical changes made.	Use a more specific title that describes the main changes, such as 'Improve chart time parsing and normalization for mixed timestamp formats' or 'Add compact period and Unix timestamp parsing to chart explorer'.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
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.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/chart-review-debt-followup

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[gemini-code-assist]

Code Review

This pull request refactors time parsing and sorting in chart-explorer.js to support compact date formats (e.g., YYYYMMDD) and handle datasets with mixed intraday and daily timestamps. It introduces several helper functions, including parseCompactPeriod, parseUnixTimestamp, and normalizeMixedTimes, to improve code modularity and data consistency. Feedback suggests optimizing the sorting logic in normalizeData to avoid redundant Date object creation by leveraging lexicographical comparison for ISO-formatted date strings when intraday data is not present.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9f1916209d

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2f45f89335

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: faca4e0158

ℹ️ 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".