Skip to content

Add price metric and longer timeframes to stock chart - #244

Open
dbigham wants to merge 2 commits into
mainfrom
codex/github-mention-add-price-metric-and-longer-timeframes-to-st
Open

Add price metric and longer timeframes to stock chart#244
dbigham wants to merge 2 commits into
mainfrom
codex/github-mention-add-price-metric-and-longer-timeframes-to-st

Conversation

@dbigham

@dbigham dbigham commented Nov 16, 2025

Copy link
Copy Markdown
Owner

Summary

  • add a Price option to the Total P&L chart when viewing a symbol and neutralize its hover formatting
  • include daily price values in the symbol total P&L series so charts can render the new metric
  • extend chart timeframe controls with 5Y and 10Y support and update shared utilities accordingly
  • remove the account-level price lookup that referenced an undefined targetSymbol so total P&L calculations no longer throw a ReferenceError

Testing

  • Not Run (not requested)

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

const sortedDates = Array.from(totalsByDate.keys()).sort();
const combinedPoints = sortedDates
.map((dateKey) => {
const bucket = totalsByDate.get(dateKey);
return {
date: dateKey,
equityCad: bucket && bucket.equityCount > 0 ? bucket.equity : undefined,
cumulativeNetDepositsCad: bucket && bucket.depositsCount > 0 ? bucket.deposits : undefined,
totalPnlCad: bucket && bucket.pnlCount > 0 ? bucket.pnl : undefined,

P1 Badge Aggregated symbol series omit price fields

The aggregation path drops priceCad/priceNative when building combined series, so responses for symbol views that span multiple accounts no longer contain any price data. The chart code relies on those fields to populate the new Price metric and, when they are missing, it falls back to the original P&L values, rendering a "Price" chart that actually shows P&L. Include price values in the aggregated points (and summary) so the Price option works for aggregated symbol views.

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

@dbigham

dbigham commented Nov 16, 2025

Copy link
Copy Markdown
Owner Author

@codex , can you fix it?

@chatgpt-codex-connector

Copy link
Copy Markdown

Summary

  • Aggregate priceCad/priceNative values alongside equity, deposits, and P&L so multi-account symbol series now propagate price data into each combined point, allowing the chart’s Price metric to render correctly in aggregated views. server/src/index.jsL420-L612
  • Include the averaged aggregated price in the summary payload (with a fallback to the final combined point) so symbol summaries also expose the Price metric for multi-account selections. server/src/index.jsL620-L644

Testing

  • Not Run (not requested)

View task →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant