Fix ATAS localized number parsing and equity refresh

[cursor]

Summary

• Fix ATAS imports to parse European-formatted numeric strings such as 12,50, 1 234,56, and currency-formatted PnL without corrupting prices/PnL.
• Refresh equity chart data after trade refreshes, imports, direct updates, and deletes when the equity widget is present.

Bug and Impact

• French/European ATAS workbooks could silently import incorrect PnL and prices because comma decimals were stripped as thousands separators.
• The equity chart could remain stale after trade imports, edits, or deletes because the centralized equity data fetch was only triggered by initial/filter loads or full refreshes.

Root Cause

• ATAS processing assumed US-style numeric formatting after XLSX display formatting converted cells to locale-formatted strings.
• Trade mutation paths updated trade state/cache without refreshing the separate equity chart data store.

Validation

• npm run typecheck passes.
• Targeted npx eslint on changed files was attempted and is blocked by pre-existing react-hooks/set-state-in-effect errors in atas-processor.tsx plus unrelated warnings.

  

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
deltalytix	Ready	Preview, Comment	May 29, 2026 11:09am

[github-actions]

🤖 Automated Code Review

PR Review Summary

This PR has two themes: locale-aware ATAS number parsing (atas-processor.tsx) and equity chart refresh after trade mutations (data-provider.tsx). Overall direction is solid; the ATAS parsing fix addresses real import bugs. No critical security issues found. A few behavioral edge cases and consistency gaps are worth addressing.

---

What works well

ATAS import (atas-processor.tsx)

• parseAtasNumber is a clear upgrade over parseFloat + comma stripping. It handles European/US formats, parentheses negatives, trailing -, and NBSP/thousand separators.
• shouldSkipTrade fixes a real bug: return inside headers.forEach only exited the inner callback, so rows with invalid PnL could still be imported (often with pnl: 0). Skipping at the row level is correct.
• Reusing parseAtasNumber for quantity, volume, and prices keeps parsing consistent.

Data provider (data-provider.tsx)

• refreshEquity with default true, and refreshAllData passing refreshEquity: false, avoids duplicate equity fetches.
• Refreshing equity on the dev IndexedDB cache path fixes stale charts.
• Gating on hasEquityChartWidget limits unnecessary API calls.
• Dependency arrays for useCallback are updated appropriately.

---

Issues

Medium — invalid prices silently become "0"

Invalid or unparseable prices are coerced to "0" instead of skipping the row (unlike PnL, which skips):

            case "entryPrice":
            case "closePrice":
              if (cellValue) {
                const price = parseAtasNumber(cellValue);
                item[key] = price === undefined ? "0" : String(price);
              } else {
                item[key] = "0";

That can import trades with zero prices without user feedback. Consider skipping the row (like PnL) or surfacing a warning in the preview.

Medium — no unit tests for parsing logic

parseAtasNumber, getDecimalSeparator, and hasGroupedThousands are non-trivial and have no tests in this PR. Worth covering cases such as:

• 1.234,56 / 1,234.56
• (123,45) / 123-
• Thousands-only: 1.234 → 1234
• Ambiguous: 12.345 (decimal vs grouped thousands)
• Invalid / empty inputs

Low — ambiguous grouped-thousands heuristic

hasGroupedThousands treats 12.345 as 12345 when the fractional part is exactly three digits. That may be wrong if ATAS ever exports three decimal places. Unlikely for prices/volumes, but worth documenting or tightening if you see bad imports.

Low — inconsistent isSharedView guard for equity refresh

refreshTradesOnly uses:

      const shouldRefreshEquity =
        refreshEquity && !isSharedView && hasEquityChartWidget;

updateTrades and deleteTrades only check hasEquityChartWidget. If those mutations can run in shared view, you may trigger equity fetches the mount effect intentionally skips. Align guards if shared views can mutate trades.

Low — formatPnl typing vs runtime

formatPnl is typed as string | undefined, but cellValue from CSV can be a number. It works because parseAtasNumber accepts unknown, but widening the parameter type (or normalizing at the call site) would match usage.

Low — updateTrades optimistic update without rollback

setTrades runs before updateTradesAction; if the action fails, local state and the equity chart can diverge. Pre-existing; equity refresh makes stale chart data slightly more visible. Consider try/catch + revert or refreshTradesOnly on failure.

---

Performance

• Extra fetchEquityChartData on update/delete is reasonable when the equity widget is present.
• fetchEquityChartData depends on many filters, so callbacks re-create often; acceptable, but rapid mutations could overlap requests (no debounce). Only worth optimizing if you see flicker or load issues.

---

Security

No new obvious risks: client-side parsing, no eval, no unsafe HTML. User-supplied CSV input is handled with bounded string operations.

---

Verdict

Approve with minor follow-ups. The PnL skip fix and locale-aware parsing are valuable. Before merge, consider at least:

1. Aligning price error handling with PnL (skip or warn instead of silent "0").
2. Adding unit tests for parseAtasNumber (and helpers).

No critical blockers; the main product risk is silently importing zero-price trades when parsing fails.

---

Generated by Cursor CLI workflow