New chart for spice auctions plus misc changes#1251
Open
marshall2112 wants to merge 10 commits intostagefrom
Open
New chart for spice auctions plus misc changes#1251marshall2112 wants to merge 10 commits intostagefrom
marshall2112 wants to merge 10 commits intostagefrom
Conversation
marshall2112
force-pushed
the
spice-dot-chart
branch
from
e254dd3 to
9a3285e
Compare
📝 WalkthroughWalkthroughReplaces the single legacy Chart with a ChartWrapper that provides a chart-type selector and per-chart filters. Adds BidHistoryChart and SpiceFinalPriceChart components, a generic DotChart, shared chart types/utilities (types, bid aggregation, Y-axis helpers), and a new useBidHistory hook that pages spice bid history from subgraphs and buckets bids into 12‑hour intervals. Updates useClosingPriceHistory date formatting and subgraph query utilities to support bid history fields. Removes old Chart and BarChart implementations, consolidates chart exports in a Charts index, and wires filter option plumbing between ChartWrapper and child charts; Details now renders Charts. Sequence Diagram(s)sequenceDiagram
participant User
participant ChartWrapper
participant BidHistoryChart
participant SpiceFinalPriceChart
participant useBidHistory
participant useClosingPriceHistory
participant Subgraph
participant DotChart
User->>ChartWrapper: open Details (auctionAddress, auctionTokenAddress)
ChartWrapper->>BidHistoryChart: render (default) or SpiceFinalPriceChart on switch
alt Bid History selected
BidHistoryChart->>useBidHistory: fetch(auctionTokenAddress)
useBidHistory->>Subgraph: spiceBidHistoryQuery (paged)
Subgraph-->>useBidHistory: bidTransactions
useBidHistory-->>BidHistoryChart: BidHistoryMetrics[]
BidHistoryChart->>ChartWrapper: onFilterOptionsChange(epochOptions)
BidHistoryChart->>DotChart: render(aggregated bucketed data)
end
alt Spice Final Price selected
SpiceFinalPriceChart->>useClosingPriceHistory: fetch(auctionAddress)
useClosingPriceHistory->>Subgraph: closing price query
Subgraph-->>useClosingPriceHistory: metrics
useClosingPriceHistory-->>SpiceFinalPriceChart: metrics[]
SpiceFinalPriceChart->>ChartWrapper: onFilterOptionsChange(auctionOptions)
SpiceFinalPriceChart->>ChartWrapper: render(bar chart)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx`:
- Around line 55-67: The code currently computes selectedEpoch from
selectedFilters and then finds epochData via historyData.find, but if the parent
filter points to an old epoch not present in historyData the component returns
"No bids found"; update the logic in BidHistoryChart so that after computing
selectedEpoch you check whether historyData contains that epoch and, if not,
fallback to the most recent epoch (historyData[historyData.length - 1].epoch)
and re-fetch epochData; also guard against empty historyData before accessing
historyData[historyData.length - 1] and only render NoDataContainer when
historyData is empty or the resolved epochData has no bids, referencing
selectedFilters, selectedEpoch, historyData, epochData and NoDataContainer to
locate where to change the code.
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/dummyBidHistoryData.ts`:
- Around line 401-615: The epoch objects (e.g., the epoch '5' and '6' objects
containing fields epoch, epochLabel, startTime, endTime and the bids array)
contain bid entries whose timestamp values fall outside their epoch's
startTime/endTime; update the data so every bid.timestamp lies within its parent
epoch's startTime..endTime window by either (A) trimming/removing bids whose
timestamp is outside the epoch range in the bids arrays, or (B) adjusting the
epoch's endTime (or startTime) to encompass the intended bids—apply the same
correction pattern for all epoch objects referenced (including the block
starting with epoch '5' and the later epochs around lines 611-918).
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsx`:
- Around line 63-71: selectedFilters may be stale (from a previous auction)
causing chartData to be empty; compute activeFilters by first intersecting
selectedFilters with current auctionOptions (match on option.value) and if that
intersection is empty use auctionOptions as the fallback, then use that
activeFilters when filtering metrics to build chartData; update the logic around
selectedFilters, auctionOptions, activeFilters, and chartData (and keep the
existing timestamp-to-string comparison) so filters are always limited to
current auctionOptions.
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx`:
- Line 96: The inline comment near the bucket processing in use-bid-history.tsx
is incorrect (it says 12-hour buckets) while the code uses BUCKET_SIZE_HOURS =
24; update the comment to say 24-hour grouping to match the BUCKET_SIZE_HOURS
constant and the bucketing logic in the function that processes each epoch's
bids (look for the // Process each epoch's bids into ... comment and the
BUCKET_SIZE_HOURS symbol).
In `@apps/dapp/src/utils/subgraph.ts`:
- Around line 799-834: The spiceBidHistoryQuery currently omits pagination and
will truncate large results; update the spiceBidHistoryQuery signature to accept
pagination knobs (e.g., pageSize/first and skip/offset or page & pageSize) and
include those variables in the GraphQL payload (use first: $first and skip:
$skip or compute skip from page) so the query requests a paged slice of
bidTransactions, and document that callers of spiceBidHistoryQuery (and any code
using SubGraphQuery<SpiceBidHistoryResp>) must loop/iterate pages until fewer
than pageSize items are returned to fetch the full history.
- Around line 631-639: The current guard in subgraph.ts that checks
rawResults.errors !== undefined incorrectly treats an empty errors array as a
failure; update the check to only throw when there are actual errors (e.g.,
(rawResults.errors || []).length > 0) so valid responses with errors: [] pass,
locate the block using rawResults and label and replace the conditional and
throw logic accordingly to only log/throw when the errors array has entries.
🧹 Nitpick comments (1)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/bidHistoryYAxisDomain.ts (1)
14-22: Consider rounding ticks to avoid noisy labels.
Unrounded ticks can render long decimals on the axis. A small rounding step keeps labels readable without changing the domain logic.♻️ Suggested tweak
- for (let i = 0; i < tickCount; i++) { - ticks.push((domainMax / (tickCount - 1)) * i); - } + for (let i = 0; i < tickCount; i++) { + const rawTick = (domainMax / (tickCount - 1)) * i; + ticks.push(Number(rawTick.toFixed(4))); + }
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx
Outdated
Show resolved
Hide resolved
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/dummyBidHistoryData.ts
Outdated
Show resolved
Hide resolved
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsx
Show resolved
Hide resolved
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/bidHistoryYAxisDomain.ts`:
- Around line 9-22: The current logic computes max and domainMax from values and
can yield a zero-height domain when all entries are 0; update the domainMax
calculation so that after const max = Math.max(...values) you check if max === 0
and, in that case, set domainMax to a small positive fallback (e.g. 1) instead
of max * 1.05, then proceed to generate the five ticks using tickCount and ticks
as before so the Y domain is never [0,0] and still produces 5 evenly spaced
ticks.
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/ChartWrapper.tsx`:
- Around line 41-55: The component keeps chart filter state across auction
changes causing stale options; add a useEffect that listens to auctionAddress
and auctionTokenAddress and resets state: call
setSelectedChartType(chartTypeOptions[0]), setChartSpecificFilters([]),
setSpiceFinalPriceOptions([]) and setBidHistoryOptions([]) when either
auctionAddress or auctionTokenAddress changes so filters/options are cleared for
the new auction.
🧹 Nitpick comments (1)
apps/dapp/src/constants/newenv/test.ts (1)
293-299: Minor naming inconsistency.The
namefield is'SPICE'(all caps), butSPICE_TOKEN_ON_BERACHAIN_BEPOLIAat line 121 uses'Spice'(title case). Other tokens also use title case (e.g.,'Temple Gold'). Consider aligning for consistency if this value is displayed in the UI.🔧 Suggested fix
const SPICE_TOKEN_ON_ETH_SEPOLIA: TokenConfig = { chainId: ETH_SEPOLIA.id, - name: 'SPICE', + name: 'Spice', address: '0x34772E1683288fC2671BbfB4FD5B705ae04e5c29', decimals: 18, symbol: 'SPICE', };
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/bidHistoryYAxisDomain.ts
Outdated
Show resolved
Hide resolved
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/ChartWrapper.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dapp/src/components/InputSelect/InputSelect.tsx (1)
12-19:⚠️ Potential issue | 🟡 MinorRemove the duplicate
valueproperty inSelectTempleDaoProps.The property
value?: Option;is declared twice at lines 16 and 18. While TypeScript silently merges identical declarations, this redundancy is confusing and violates best practices for application interfaces. Declaration merging is intended for module augmentation, not for regular code. Keep a single declaration.Suggested fix
export interface SelectTempleDaoProps { onChange?(event: any): void; options: SelectTempleDaoOptions | []; value?: Option; defaultValue?: Option; - value?: Option; // use to limit the number of elements shown in the menu at anytime maxMenuItems?: number;
marshall2112
force-pushed
the
spice-dot-chart
branch
from
679f6af to
2d71a95
Compare
marshall2112
added
the
preview-deploy
Collaborator
Author
|
Deploy preview for core ready! ✅ Preview Built with commit 35bf3f9. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx`:
- Around line 14-15: The bucket comment examples are incorrect for
BUCKET_SIZE_HOURS = 12; update the comments for the `bucket` field (and related
examples near `bucketIndex`) in use-bid-history.tsx to reflect 12-hour grouping
(e.g., "0-12h", "12-24h", "24-36h", etc.) and ensure any example values for
`bucketIndex` remain consistent with those buckets; reference the
`BUCKET_SIZE_HOURS` usage and the `bucket` and `bucketIndex` identifiers when
updating the inline comments.
🧹 Nitpick comments (2)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx (1)
132-132: Remove unusedindexparameter.The
indexparameter in the map callback is declared but never used.✏️ Suggested fix
- const processedBids: BidData[] = bids.map((bid: any, index: number) => { + const processedBids: BidData[] = bids.map((bid: any) => {apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx (1)
37-41: Potential re-render loop ifonFilterOptionsChangeis not memoized.If the parent component passes a non-memoized callback (inline function), this
useEffectwill run on every render, potentially causing unnecessary re-renders or loops. Consider documenting this expectation or adding a ref-based guard.🛠️ Safer alternative using functional comparison
+import { useRef } from 'react'; + // Expose available options to parent component +const prevOptionsRef = useRef<Option[]>([]); useEffect(() => { - if (epochOptions.length > 0 && onFilterOptionsChange) { + const optionsChanged = + epochOptions.length !== prevOptionsRef.current.length || + epochOptions.some((opt, i) => opt.value !== prevOptionsRef.current[i]?.value); + + if (epochOptions.length > 0 && onFilterOptionsChange && optionsChanged) { + prevOptionsRef.current = epochOptions; onFilterOptionsChange(epochOptions); } }, [epochOptions, onFilterOptionsChange]);
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/dapp/src/utils/subgraph.ts (1)
2-5: Unused imports.
nullable(line 2) andtimeStamp(line 5) are imported but not used in the file.Suggested cleanup
-import { nullable, z } from 'zod'; +import { z } from 'zod'; import { backOff } from 'exponential-backoff'; import { SpiceAuction } from 'types/typechain'; -import { timeStamp } from 'console'; import { useQuery, QueryClient } from '@tanstack/react-query';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/utils/subgraph.ts` around lines 2 - 5, The file imports unused symbols nullable and timeStamp which should be removed to clean up imports; edit the imports at the top of apps/dapp/src/utils/subgraph.ts to drop nullable from "zod" and timeStamp from "console" so only used symbols (e.g., z, backOff, SpiceAuction) remain, then run lint/type-check to confirm no other references to nullable or timeStamp exist.apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx (1)
27-47:CustomDotshould be typed (and guardpayload) instead of usingany.
Right now, a missing/undefinedpayloadwould throw, andanyhides shape/prop mismatches.Proposed fix (minimal local typing, no Recharts-type imports required)
-const CustomDot = (props: any) => { +type CustomDotProps = { + cx?: number; + cy?: number; + payload?: Record<string, any>; + highlightKey?: string; + dotColor: string; + highlightColor: string; + fixedRadius?: number; +}; + +const CustomDot = (props: CustomDotProps) => { const { cx, cy, payload, highlightKey, dotColor, highlightColor, fixedRadius, } = props; - const isHighlighted = highlightKey ? payload[highlightKey] : false; + const isHighlighted = !!(highlightKey && payload?.[highlightKey]); const fill = isHighlighted ? highlightColor : dotColor; const r = - fixedRadius ?? getScaledRadius(payload.count ?? 1, payload.maxCount ?? 1); + fixedRadius ?? + getScaledRadius(payload?.count ?? 1, payload?.maxCount ?? 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx` around lines 27 - 47, CustomDot currently uses props: any and accesses payload without guarding, which can throw; replace any with a small local Props interface (including cx?: number, cy?: number, payload?: Record<string, any> | undefined, highlightKey?: string, dotColor?: string, highlightColor?: string, fixedRadius?: number) and update the function signature to use that type; add a defensive guard at the start of CustomDot that returns null (or an empty <g/>) when payload is falsy before reading payload[highlightKey], payload.count or payload.maxCount; keep using getScaledRadius and the existing prop names (cx, cy, highlightKey, dotColor, highlightColor, fixedRadius) so callers remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx`:
- Around line 72-82: The current logic coerces invalid X values to 0 via
Number(d[xDataKey]) || 0 which can silently skew the domain; update the
dataIndices computation to convert values with Number(d[xDataKey]) but exclude
non-finite results (e.g., filter out isNaN/!isFinite) instead of defaulting to
0, similarly ensure tickIndices already filters NaN, then compute allIndices
from these filtered arrays and keep the existing fallback for minBucketIndex and
maxBucketIndex (use Math.min/Math.max only when allIndices.length > 0) so
malformed Xs are ignored rather than turned into zeros; refer to symbols
chartData, xDataKey, xTicks, dataIndices, tickIndices, allIndices,
minBucketIndex, and maxBucketIndex.
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/yAxisUtils.ts`:
- Around line 78-96: When values is non-empty but all zeros, max becomes 0
producing a degenerate yDomain and identical yTicks; update the logic in
yAxisUtils (the block computing max, domainMax, tickCount and ticks from values)
to guard when max === 0 by setting a sensible non-zero domainMax (for example 1
or a small positive default) and then generate five evenly spaced ticks from 0
to that domainMax so yDomain and yTicks are non-degenerate even for all-zero
inputs.
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx`:
- Around line 36-41: The effect in BidHistoryChart currently only notifies the
parent when epochOptions.length > 0, leaving the parent with stale filters when
epochOptions becomes empty; update the useEffect that references epochOptions
and onFilterOptionsChange to call onFilterOptionsChange(epochOptions) whenever
epochOptions changes (remove the length > 0 check) while still guarding that
onFilterOptionsChange is defined so the parent receives empty arrays and can
clear stale selections.
- Around line 82-99: The code uses parseFloat(bid.bidAmount) directly which can
produce NaN and corrupt bucket totals; compute a numeric amount once (e.g.,
const amount = parseFloat(bid.bidAmount)) and guard it with
Number.isFinite(amount) ? amount : 0, then use that guarded value when setting
totalBidAmount in the new bucket and when adding to existing.totalBidAmount in
the epochData.bids loop (keep bucketMap, bid.bucketIndex, bid.bucket, prices,
count, and isFinalBid logic unchanged).
---
Duplicate comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx`:
- Around line 55-67: selectedEpoch resolution is brittle:
selectedFilters[0].value can be undefined or a mismatched type and may not exist
in historyData, causing epochData to be undefined and incorrectly showing "No
bids found". Fix by resolving selectedEpoch defensively in BidHistoryChart: read
candidate = selectedFilters?.[0]?.value, coerce/validate it (e.g.,
Number(candidate) or ensure typeof candidate === 'number'), check that
historyData includes an entry with epoch === candidate, and if not use the
latest epoch from historyData (guarding for empty historyData first). Then use
historyData.find(...) to get epochData; if historyData is empty return
NoDataContainer as before. This change touches selectedFilters, selectedEpoch,
historyData, epochData handling in BidHistoryChart.
---
Nitpick comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx`:
- Around line 27-47: CustomDot currently uses props: any and accesses payload
without guarding, which can throw; replace any with a small local Props
interface (including cx?: number, cy?: number, payload?: Record<string, any> |
undefined, highlightKey?: string, dotColor?: string, highlightColor?: string,
fixedRadius?: number) and update the function signature to use that type; add a
defensive guard at the start of CustomDot that returns null (or an empty <g/>)
when payload is falsy before reading payload[highlightKey], payload.count or
payload.maxCount; keep using getScaledRadius and the existing prop names (cx,
cy, highlightKey, dotColor, highlightColor, fixedRadius) so callers remain
unchanged.
In `@apps/dapp/src/utils/subgraph.ts`:
- Around line 2-5: The file imports unused symbols nullable and timeStamp which
should be removed to clean up imports; edit the imports at the top of
apps/dapp/src/utils/subgraph.ts to drop nullable from "zod" and timeStamp from
"console" so only used symbols (e.g., z, backOff, SpiceAuction) remain, then run
lint/type-check to confirm no other references to nullable or timeStamp exist.
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx
Show resolved
Hide resolved
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/yAxisUtils.ts
Show resolved
Hide resolved
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx
Show resolved
Hide resolved
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx
Outdated
Show resolved
Hide resolved
marshall2112
force-pushed
the
spice-dot-chart
branch
from
50d4952 to
1e04415
Compare
Collaborator
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx (1)
14-15:⚠️ Potential issue | 🟡 MinorUpdate bucket examples to match 12-hour bucketing.
On Line 14 and Line 15, the examples still describe 24-hour windows, but
BUCKET_SIZE_HOURSis 12.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx` around lines 14 - 15, Update the inline examples for the bucket fields to reflect the 12-hour bucketing used by BUCKET_SIZE_HOURS: change the comment for bucket to show ranges like "0-12h", "12-24h" instead of "0-24h", and update bucketIndex example to indicate it maps to 12-hour windows (e.g., 0, 1, 2...) so the comments near the bucket and bucketIndex declarations in use-bid-history.tsx align with BUCKET_SIZE_HOURS = 12.apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsx (1)
63-70:⚠️ Potential issue | 🟡 MinorIntersect selected filters with current options before filtering data.
On Line 65, stale
selectedFiltersfrom a previous context can zero outchartDataeven when metrics exist. Intersect withauctionOptionsfirst, then fallback toauctionOptionsif intersection is empty.Suggested fix
- const activeFilters = - selectedFilters.length > 0 ? selectedFilters : auctionOptions; + const validSelectedFilters = selectedFilters.filter((f) => + auctionOptions.some((o) => o.value === f.value) + ); + const activeFilters = + validSelectedFilters.length > 0 ? validSelectedFilters : auctionOptions;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsx` around lines 63 - 70, selectedFilters may contain stale entries not present in auctionOptions which can make chartData empty; compute an intersection of selectedFilters with auctionOptions first (e.g., filter selectedFilters by existence in auctionOptions using matching value), then set activeFilters to that intersection if non-empty, otherwise fallback to auctionOptions, and use activeFilters in the metrics filter (references: selectedFilters, auctionOptions, activeFilters, chartData, metrics).apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx (1)
73-81:⚠️ Potential issue | 🟡 MinorDon’t coerce invalid X values to
0when building the X domain.On Line 73,
Number(...) || 0can silently inject zeros for malformed values and distort axis bounds. Filter to finite numbers instead.Suggested fix
- const dataIndices = chartData.map((d: any) => Number(d[xDataKey]) || 0); - const tickIndices = (xTicks || []) + const dataIndices = chartData + .map((d: any) => Number(d[xDataKey])) + .filter((num) => Number.isFinite(num)); + const tickIndices = (xTicks || []) .map((tick) => Number(tick)) - .filter((num) => !isNaN(num)); + .filter((num) => Number.isFinite(num));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx` around lines 73 - 81, The current mapping for dataIndices uses Number(d[xDataKey]) || 0 which coerces non-numeric X values to 0 and can skew the domain; change the dataIndices and tickIndices creation to map to Number(...) and then filter using Number.isFinite to keep only valid finite numbers (e.g., dataIndices = chartData.map(d => Number(d[xDataKey])).filter(Number.isFinite)), likewise filter xTicks into tickIndices, then compute allIndices from these filtered arrays and derive minBucketIndex/maxBucketIndex with the same empty-array fallback (allIndices.length ? Math.min(...allIndices) : 0 and Math.max(...allIndices) : 1) so malformed X values are excluded.apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/yAxisUtils.ts (1)
81-95:⚠️ Potential issue | 🟡 MinorHandle all-zero bid values to avoid a degenerate Y-axis.
On Line 84,
domainMaxbecomes0when all values are zero, producingyDomain: [0, 0]and duplicate ticks. Add a zero-max guard before generating ticks.Suggested fix
const max = Math.max(...values); + + if (max === 0) { + return { yDomain: [0, 1], yTicks: [0, 0.25, 0.5, 0.75, 1] }; + } // Domain: start at 0, end 5% above max const domainMax = max * 1.05;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/yAxisUtils.ts` around lines 81 - 95, The y-axis calculation produces a degenerate domain when all bid values are zero (max === 0) causing yDomain [0,0] and duplicate ticks; update the logic in yAxisUtils.ts around the const max and domainMax variables to guard against this by setting a sensible positive fallback (e.g., domainMax = 1) when max is 0 before generating ticks, then generate the 5 evenly spaced ticks (using tickCount) from 0 to domainMax so yDomain and yTicks are valid even for all-zero input.apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx (3)
37-41:⚠️ Potential issue | 🟡 MinorPropagate empty
epochOptionsto clear stale parent filters.At Line 38, gating on
epochOptions.length > 0can leave stale selection state in the parent when history becomes empty.Proposed fix
useEffect(() => { - if (epochOptions.length > 0 && onFilterOptionsChange) { - onFilterOptionsChange(epochOptions); - } + if (!onFilterOptionsChange) return; + onFilterOptionsChange(epochOptions); }, [epochOptions, onFilterOptionsChange]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx` around lines 37 - 41, The effect in BidHistoryChart.tsx currently guards calling onFilterOptionsChange with epochOptions.length > 0 which prevents propagating an empty epochOptions to the parent and can leave stale filters; update the useEffect (the effect that depends on [epochOptions, onFilterOptionsChange]) to call onFilterOptionsChange(epochOptions) unconditionally whenever epochOptions changes so the parent receives an empty array to clear filters instead of skipping the update.
57-67:⚠️ Potential issue | 🟡 MinorFallback to latest epoch when selected filter no longer exists.
At Line 57, if the selected epoch is stale/missing, the chart incorrectly drops to “No bids found” despite available data.
Proposed fix
- const selectedEpoch = - selectedFilters.length > 0 - ? selectedFilters[0].value - : historyData[historyData.length - 1].epoch; + const latestEpoch = historyData[historyData.length - 1].epoch; + const selectedEpoch = + historyData.find((d) => d.epoch === selectedFilters[0]?.value)?.epoch ?? + latestEpoch; // Find the selected epoch's data const epochData = historyData.find((data) => data.epoch === selectedEpoch);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx` around lines 57 - 67, The current logic sets selectedEpoch from selectedFilters[0].value without verifying it exists in historyData, causing epochData to be undefined and the chart to show "No bids found" even when newer data exists; update the selection logic (variables: selectedEpoch, selectedFilters, historyData, epochData) to check whether selectedFilters[0].value exists in historyData (e.g., compare against historyData.map(d => d.epoch)) and if it does not, fall back to the latest epoch from historyData before computing epochData, so the component renders the most recent available bids instead of the NoDataContainer.
82-95:⚠️ Potential issue | 🟡 MinorGuard parsed bid amounts to prevent
NaNaggregation.At Line 86 and Line 94,
parseFloatcan returnNaN, which poisonstotalBidAmountfor the bucket.Proposed fix
epochData.bids.forEach((bid) => { + const parsedBidAmount = parseFloat(bid.bidAmount); + const safeBidAmount = Number.isFinite(parsedBidAmount) + ? parsedBidAmount + : 0; + const existing = bucketMap.get(bid.bucketIndex); if (existing) { existing.prices.push(bid.price); - existing.totalBidAmount += parseFloat(bid.bidAmount); + existing.totalBidAmount += safeBidAmount; existing.count += 1; if (bid.isFinalBid) existing.isFinalBid = true; } else { bucketMap.set(bid.bucketIndex, { bucket: bid.bucket, bucketIndex: bid.bucketIndex, prices: [bid.price], - totalBidAmount: parseFloat(bid.bidAmount), + totalBidAmount: safeBidAmount, count: 1, isFinalBid: bid.isFinalBid, }); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx` around lines 82 - 95, The code uses parseFloat(bid.bidAmount) when updating bucket.totalBidAmount (in the bucketMap update and when creating a new bucket), which can produce NaN and corrupt aggregations; fix by parsing bid.bidAmount into a numeric value once (e.g., const amount = Number.parseFloat(bid.bidAmount)) and guard it with Number.isFinite (or isNaN) to fall back to 0 when invalid, then use that sanitized amount when adding to existing.totalBidAmount and when setting totalBidAmount on the new bucket; locate uses of parseFloat in the epochData.bids loop, particularly where totalBidAmount is set or incremented, and replace with the guarded parsed value.
🧹 Nitpick comments (2)
apps/dapp/src/constants/newenv/test.ts (1)
27-30: Fix the typo in the commented constant name.Line 27 uses
ARBIBRUMin the commented constant; this should beARBITRUMto avoid confusion when re-enabling or searching configs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/constants/newenv/test.ts` around lines 27 - 30, Update the commented constant name ARBIBRUM_SPICE_BAZAAR_SUBGRAPH_URL to the correct spelling ARBITRUM_SPICE_BAZAAR_SUBGRAPH_URL in the file so searches and future re-enabling use the correct identifier (leave the URL/comment block unchanged other than renaming the symbol).apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/ChartWrapper.tsx (1)
243-257: Remove unused placeholder styled components (or wire them in when needed).
EmptyChartContainerandEmptyChartTextare currently dead code in this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/ChartWrapper.tsx` around lines 243 - 257, The file declares two unused styled components, EmptyChartContainer and EmptyChartText; remove these dead declarations to clean up the file (or if they are intended to be shown when chart data is absent, wire them into the ChartWrapper render path by replacing current empty/placeholder UI with these components and their text). Locate the styled declarations for EmptyChartContainer and EmptyChartText and either delete both declarations or add them into the ChartWrapper JSX conditional that renders when there is no chart data so they’re actually used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dapp/src/components/InputSelect/InputSelect.tsx`:
- Around line 16-17: The prop types for the InputSelect component currently
declare value and defaultValue as only Option which prevents representing an
empty selection; update those prop declarations (value and defaultValue in the
InputSelect props) to allow nullable values by using Option | null for
single-select mode (or import and use react-select's ValueType<Option, false>)
so controlled components can represent "no selection." Ensure any usages and
default props accommodate null where appropriate.
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx`:
- Around line 47-49: The current epoch-options build mutates auctionsData by
calling .sort(...) directly; change the code that generates the epoch options to
sort a shallow copy (e.g., use [...auctionsData] or auctionsData.slice() before
.sort) so the original auctionsData array is not mutated; update the map chain
that starts with auctionsData.sort(...) in Chart2.tsx to operate on a copied
array instead.
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsx`:
- Around line 45-49: The effect currently only calls onFilterOptionsChange when
auctionOptions.length > 0, leaving the parent stale when options become empty;
change the useEffect to call onFilterOptionsChange unconditionally (as long as
the callback exists) by replacing the conditional to check only for
onFilterOptionsChange and passing auctionOptions directly to
onFilterOptionsChange from within the useEffect (referencing useEffect,
auctionOptions, and onFilterOptionsChange).
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx`:
- Around line 63-109: The loop over endpoints currently swallows all endpoint
errors and returns an empty array (via setData([])), masking fetch failures as
"no bids"; change use-bid-history.tsx to track endpoint-level failures (e.g.,
add a boolean like anyEndpointSucceeded and/or capture the lastEndpointError
when catching endpointError inside the for (const entry of endpoints) loop), and
after the loop if bidTransactions is empty and no endpoint succeeded call the
component's error handler (e.g., setError / setFetchFailed / throw the captured
error) instead of setData([]) so callers can distinguish “no bids” from “request
failure”; make sure to reference the same symbols used in the file (endpoints,
entry.url, bidTransactions, setData) and only set data to [] when an endpoint
succeeded and returned zero bids.
---
Duplicate comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx`:
- Around line 73-81: The current mapping for dataIndices uses
Number(d[xDataKey]) || 0 which coerces non-numeric X values to 0 and can skew
the domain; change the dataIndices and tickIndices creation to map to
Number(...) and then filter using Number.isFinite to keep only valid finite
numbers (e.g., dataIndices = chartData.map(d =>
Number(d[xDataKey])).filter(Number.isFinite)), likewise filter xTicks into
tickIndices, then compute allIndices from these filtered arrays and derive
minBucketIndex/maxBucketIndex with the same empty-array fallback
(allIndices.length ? Math.min(...allIndices) : 0 and Math.max(...allIndices) :
1) so malformed X values are excluded.
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/yAxisUtils.ts`:
- Around line 81-95: The y-axis calculation produces a degenerate domain when
all bid values are zero (max === 0) causing yDomain [0,0] and duplicate ticks;
update the logic in yAxisUtils.ts around the const max and domainMax variables
to guard against this by setting a sensible positive fallback (e.g., domainMax =
1) when max is 0 before generating ticks, then generate the 5 evenly spaced
ticks (using tickCount) from 0 to domainMax so yDomain and yTicks are valid even
for all-zero input.
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx`:
- Around line 37-41: The effect in BidHistoryChart.tsx currently guards calling
onFilterOptionsChange with epochOptions.length > 0 which prevents propagating an
empty epochOptions to the parent and can leave stale filters; update the
useEffect (the effect that depends on [epochOptions, onFilterOptionsChange]) to
call onFilterOptionsChange(epochOptions) unconditionally whenever epochOptions
changes so the parent receives an empty array to clear filters instead of
skipping the update.
- Around line 57-67: The current logic sets selectedEpoch from
selectedFilters[0].value without verifying it exists in historyData, causing
epochData to be undefined and the chart to show "No bids found" even when newer
data exists; update the selection logic (variables: selectedEpoch,
selectedFilters, historyData, epochData) to check whether
selectedFilters[0].value exists in historyData (e.g., compare against
historyData.map(d => d.epoch)) and if it does not, fall back to the latest epoch
from historyData before computing epochData, so the component renders the most
recent available bids instead of the NoDataContainer.
- Around line 82-95: The code uses parseFloat(bid.bidAmount) when updating
bucket.totalBidAmount (in the bucketMap update and when creating a new bucket),
which can produce NaN and corrupt aggregations; fix by parsing bid.bidAmount
into a numeric value once (e.g., const amount =
Number.parseFloat(bid.bidAmount)) and guard it with Number.isFinite (or isNaN)
to fall back to 0 when invalid, then use that sanitized amount when adding to
existing.totalBidAmount and when setting totalBidAmount on the new bucket;
locate uses of parseFloat in the epochData.bids loop, particularly where
totalBidAmount is set or incremented, and replace with the guarded parsed value.
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsx`:
- Around line 63-70: selectedFilters may contain stale entries not present in
auctionOptions which can make chartData empty; compute an intersection of
selectedFilters with auctionOptions first (e.g., filter selectedFilters by
existence in auctionOptions using matching value), then set activeFilters to
that intersection if non-empty, otherwise fallback to auctionOptions, and use
activeFilters in the metrics filter (references: selectedFilters,
auctionOptions, activeFilters, chartData, metrics).
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx`:
- Around line 14-15: Update the inline examples for the bucket fields to reflect
the 12-hour bucketing used by BUCKET_SIZE_HOURS: change the comment for bucket
to show ranges like "0-12h", "12-24h" instead of "0-24h", and update bucketIndex
example to indicate it maps to 12-hour windows (e.g., 0, 1, 2...) so the
comments near the bucket and bucketIndex declarations in use-bid-history.tsx
align with BUCKET_SIZE_HOURS = 12.
---
Nitpick comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/ChartWrapper.tsx`:
- Around line 243-257: The file declares two unused styled components,
EmptyChartContainer and EmptyChartText; remove these dead declarations to clean
up the file (or if they are intended to be shown when chart data is absent, wire
them into the ChartWrapper render path by replacing current empty/placeholder UI
with these components and their text). Locate the styled declarations for
EmptyChartContainer and EmptyChartText and either delete both declarations or
add them into the ChartWrapper JSX conditional that renders when there is no
chart data so they’re actually used.
In `@apps/dapp/src/constants/newenv/test.ts`:
- Around line 27-30: Update the commented constant name
ARBIBRUM_SPICE_BAZAAR_SUBGRAPH_URL to the correct spelling
ARBITRUM_SPICE_BAZAAR_SUBGRAPH_URL in the file so searches and future
re-enabling use the correct identifier (leave the URL/comment block unchanged
other than renaming the symbol).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a226627-b828-4bb1-a788-259a53edda46
📒 Files selected for processing (20)
apps/dapp/src/components/InputSelect/InputSelect.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/BidHistoryTable/Table.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/BarChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/hooks/use-bids-history.tsapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/hooks/use-stableGold-auction-metrics.tsapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/Chart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/ChartWrapper.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Details/Details.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-closing-price-history.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/BarChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/index.tsapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/types.tsapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/yAxisUtils.tsapps/dapp/src/constants/newenv/test.tsapps/dapp/src/utils/subgraph.ts
💤 Files with no reviewable changes (2)
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/Chart.tsx
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/BarChart.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/BidHistoryTable/Table.tsx
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/index.ts
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/types.ts
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/hooks/use-stableGold-auction-metrics.ts
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-closing-price-history.tsx
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx
Outdated
Show resolved
Hide resolved
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsx
Show resolved
Hide resolved
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx (1)
266-310:⚠️ Potential issue | 🟡 MinorPrevent premature “No bids found” before epoch selection is ready.
In bid-history mode, if
selectedEpochis not initialized yet, the current flow can render an empty-state message instead of a loading state. Gate the empty-state until epoch selection is available.💡 Proposed fix
+ const isEpochSelectionPending = + isBidHistory && epochOptions.length > 0 && !selectedEpoch; - const loading = isBidHistory ? bidsLoading : metricsLoading; + const loading = isBidHistory + ? bidsLoading || isEpochSelectionPending + : metricsLoading; if (loading || (!isBidHistory && !metrics?.length)) return <Loader />;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx` around lines 266 - 310, When in bid-history mode the component can render the "No bids found" empty state before the epoch selector is ready; modify the render logic in Chart2.tsx around isBidHistory/dotAggregated.chartData so that you first guard for epoch selection readiness (check selectedEpoch or that epochOptions is populated) and return the Loader while selectedEpoch is uninitialized, and only when selectedEpoch is set evaluate dotAggregated.chartData.length === 0 to render NoDataContainer; update the conditional flow that currently shows NoDataContainer to instead require selectedEpoch (or non-empty epochOptions) before deciding between Loader, NoDataContainer, or the chart rendering.
♻️ Duplicate comments (1)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx (1)
14-15:⚠️ Potential issue | 🟡 MinorFix bucket examples to match 12-hour grouping.
The examples on Line 14 and Line 15 still describe 24-hour ranges while
BUCKET_SIZE_HOURSis12.✏️ Proposed fix
- bucket: string; // e.g., "0-24h", "24-48h" - bucketIndex: number; // e.g., 0, 1, 2... + bucket: string; // e.g., "0-12h", "12-24h" + bucketIndex: number; // e.g., 0, 1, 2... (each index is a 12-hour window)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx` around lines 14 - 15, The bucket example comment is incorrect for the current BUCKET_SIZE_HOURS (12); update the example string in the comments near bucket and bucketIndex in use-bid-history.tsx to reflect 12-hour grouping (e.g., "0-12h", "12-24h", etc.) and adjust the explanatory text for bucketIndex if needed (e.g., 0, 1, 2...) so the comments align with BUCKET_SIZE_HOURS.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx`:
- Around line 48-52: When auctionTokenAddress is falsy the early return clears
data and loading but doesn't clear a previously set error, so update the branch
that handles !auctionTokenAddress (in the hook using auctionTokenAddress,
setData and setLoading) to also reset the error state (e.g., call setError(null)
or setError(undefined)) before returning so stale errors don't persist when
input becomes undefined.
---
Outside diff comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx`:
- Around line 266-310: When in bid-history mode the component can render the "No
bids found" empty state before the epoch selector is ready; modify the render
logic in Chart2.tsx around isBidHistory/dotAggregated.chartData so that you
first guard for epoch selection readiness (check selectedEpoch or that
epochOptions is populated) and return the Loader while selectedEpoch is
uninitialized, and only when selectedEpoch is set evaluate
dotAggregated.chartData.length === 0 to render NoDataContainer; update the
conditional flow that currently shows NoDataContainer to instead require
selectedEpoch (or non-empty epochOptions) before deciding between Loader,
NoDataContainer, or the chart rendering.
---
Duplicate comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx`:
- Around line 14-15: The bucket example comment is incorrect for the current
BUCKET_SIZE_HOURS (12); update the example string in the comments near bucket
and bucketIndex in use-bid-history.tsx to reflect 12-hour grouping (e.g.,
"0-12h", "12-24h", etc.) and adjust the explanatory text for bucketIndex if
needed (e.g., 0, 1, 2...) so the comments align with BUCKET_SIZE_HOURS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48048a2e-53e2-4a03-b999-854b50b470b8
📒 Files selected for processing (4)
apps/dapp/src/components/InputSelect/InputSelect.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dapp/src/components/InputSelect/InputSelect.tsx
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsx
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx`:
- Line 42: The component currently only guards rendering with bidsLoading, but
useAuctionsHistory() also returns a loading flag (the variable from
useAuctionsHistory is auctionsLoading while the data is auctionsData), so update
the loading guard(s) in Chart2 (where epochOptions and the "No bids found for
this epoch" message are determined) to check both bidsLoading and
auctionsLoading (e.g., treat as loading if either is true) so the spinner shows
while auction data is fetched; adjust all instances of the guard (including the
block around epochOptions and the rendering branch between lines ~205–249) to
use the combined condition.
In `@apps/dapp/src/utils/subgraph.ts`:
- Around line 802-846: The cache label for spiceBidHistoryQuery is constant
("SpiceBidHistory") so different parameter combinations collide in React Query;
change the label generation in spiceBidHistoryQuery to be parameter-aware (e.g.,
include auctionToken.toLowerCase(), first and skip) so each unique request gets
a unique label (for example "SpiceBidHistory:{auctionToken}:{first}:{skip}" or a
stable hash of the request) and return that label from the function along with
the same request and parser.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 10ce7bb2-e4b4-43cf-9a0d-1d7d25acf7bc
📒 Files selected for processing (6)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/ChartWrapper.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/bidAggregation.tsapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/index.tsapps/dapp/src/utils/subgraph.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx (1)
70-75:⚠️ Potential issue | 🟡 MinorBid History can still flash a false empty state before the initial epoch is seeded.
Lines 71-75 populate
selectedEpochin an effect, so the first bid-history render can still reach Line 82 withepochStr === undefined. IfbidsLoadingis false on that render, Lines 249-251 briefly show “No bids found for this epoch” even though the component is still initializing.Suggested fix
+ const bidHistoryInitializing = + isBidHistory && epochOptions.length > 0 && !selectedEpoch; + const loading = isBidHistory - ? bidsLoading || auctionsLoading + ? auctionsLoading || bidHistoryInitializing || bidsLoading : metricsLoading;Also applies to: 82-84, 205-206, 249-251
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx` around lines 70 - 75, The component can render a false "No bids found" state before selectedEpoch is set; update the rendering guards to check that an epoch is actually selected (e.g., ensure selectedEpoch or epochStr is defined and epochOptions.length>0) before showing the empty-state message and before treating bidsLoading as final; modify the useEffect/initialization to avoid transient undefined by either setting a temporary loading state until setSelectedEpoch runs or by short-circuiting the empty-bids branches (references: useEffect that sets selectedEpoch, selectedEpoch variable, epochStr, and the bidsLoading/empty-state render paths around where "No bids found for this epoch" is returned).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx`:
- Around line 271-284: The tooltip shows rounded numbers because
formatNumberFixedDecimals currently returns a number (dropping trailing zeros);
update the tooltip construction in Chart2.tsx to use fixed-width string values
instead of numeric values—either change formatNumberFixedDecimals to return a
string (e.g., "0.100000") and use that result for
avgFormatted/minFormatted/maxFormatted/totalFormatted, or call .toFixed(...) on
the raw numeric fields (d.price, d.totalBidAmount, d.minPrice, d.maxPrice) when
building the lines array so the interpolated values preserve trailing zeros.
---
Duplicate comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx`:
- Around line 70-75: The component can render a false "No bids found" state
before selectedEpoch is set; update the rendering guards to check that an epoch
is actually selected (e.g., ensure selectedEpoch or epochStr is defined and
epochOptions.length>0) before showing the empty-state message and before
treating bidsLoading as final; modify the useEffect/initialization to avoid
transient undefined by either setting a temporary loading state until
setSelectedEpoch runs or by short-circuiting the empty-bids branches
(references: useEffect that sets selectedEpoch, selectedEpoch variable,
epochStr, and the bidsLoading/empty-state render paths around where "No bids
found for this epoch" is returned).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c6304f83-f77a-432b-97ca-44b1921358f7
📒 Files selected for processing (2)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx (1)
78-81: Type assertion may maskundefinedvalue.When
selectedEpochis undefined (before the effect initializes it),selectedEpoch?.valueevaluates toundefined, not a string. Theas stringassertion is misleading sinceepochStrcan beundefinedat runtime.Suggested improvement
- const epochStr = isBidHistory ? (selectedEpoch?.value as string) : undefined; + const epochStr = isBidHistory ? selectedEpoch?.value : undefined;If
useBidsHistoryexpectsstring | undefined, the explicit assertion is unnecessary. If it requires astring, consider gating the call or adding early return logic whenselectedEpochis undefined.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx` around lines 78 - 81, The type assertion "as string" on selectedEpoch?.value is unsafe because selectedEpoch can be undefined; update the code so epochStr is correctly typed and passed to useBidsHistory: either remove the "as string" and let epochStr be string | undefined (preferred) so useBidsHistory receives string | undefined, or if useBidsHistory truly requires a string, gate the call by returning early or calling useBidsHistory only when selectedEpoch is defined (e.g., compute epochStr only when selectedEpoch !== undefined and pass that string). Ensure you modify the types/signature of useBidsHistory or add the early-return/gating logic around the call to useBidsHistory so no runtime undefined is asserted as a string; reference the variables epochStr, selectedEpoch, and the hook useBidsHistory when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx`:
- Around line 78-81: The type assertion "as string" on selectedEpoch?.value is
unsafe because selectedEpoch can be undefined; update the code so epochStr is
correctly typed and passed to useBidsHistory: either remove the "as string" and
let epochStr be string | undefined (preferred) so useBidsHistory receives string
| undefined, or if useBidsHistory truly requires a string, gate the call by
returning early or calling useBidsHistory only when selectedEpoch is defined
(e.g., compute epochStr only when selectedEpoch !== undefined and pass that
string). Ensure you modify the types/signature of useBidsHistory or add the
early-return/gating logic around the call to useBidsHistory so no runtime
undefined is asserted as a string; reference the variables epochStr,
selectedEpoch, and the hook useBidsHistory when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe7fa9fd-27ec-4cf5-bfcd-24773d0e25c3
📒 Files selected for processing (1)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx
marshall2112
force-pushed
the
spice-dot-chart
branch
from
b6beed8 to
35bf3f9
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx (1)
72-81:⚠️ Potential issue | 🟡 MinorIgnore invalid/non-finite X values instead of coercing to
0.Line 73 currently turns malformed X values into
0, which can distort the computed X domain and mask bad data. Also, Line 76 should filter finite values, not justNaN.Proposed fix
- const dataIndices = chartData.map((d: any) => Number(d[xDataKey]) || 0); - const tickIndices = (xTicks || []) - .map((tick) => Number(tick)) - .filter((num) => !isNaN(num)); + const dataIndices = chartData + .map((d: any) => Number(d[xDataKey])) + .filter((num) => Number.isFinite(num)); + const tickIndices = (xTicks ?? []) + .map((tick) => Number(tick)) + .filter((num) => Number.isFinite(num)); const allIndices = [...dataIndices, ...tickIndices]; - const minBucketIndex = allIndices.length > 0 ? Math.min(...allIndices) : 0; - const maxBucketIndex = allIndices.length > 0 ? Math.max(...allIndices) : 1; + const minBucketIndex = allIndices.length ? Math.min(...allIndices) : 0; + const maxBucketIndex = allIndices.length ? Math.max(...allIndices) : 1;#!/bin/bash # Verify the coercion/filter pattern in DotChart domain calculation. rg -n -C2 'Number\(d\[xDataKey\]\)\s*\|\|\s*0|filter\(\(num\)\s*=>\s*!isNaN\(num\)\)' apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx` around lines 72 - 81, The current mapping coerces malformed X values to 0 which skews the domain; update the dataIndices and tickIndices logic to only include finite numeric X values: when building dataIndices from chartData use Number(value) (where value is d[xDataKey]) and filter with Number.isFinite to drop non-finite/malformed entries, and for tickIndices map xTicks to Number and also filter with Number.isFinite (instead of !isNaN); keep the existing allIndices, minBucketIndex and maxBucketIndex fallback behavior when allIndices is empty.
🧹 Nitpick comments (1)
apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/ChartWrapper.tsx (1)
233-247: Unused styled components can be removed.
EmptyChartContainerandEmptyChartTextare defined but not currently used (theTotalTGLDBidchart case is commented out). Consider removing these until the feature is implemented to reduce dead code.🧹 Suggested cleanup
-const EmptyChartContainer = styled.div` - display: flex; - align-items: center; - justify-content: center; - min-height: 350px; - border: 1px dashed ${({ theme }) => theme.palette.brand}; - border-radius: 10px; - background: ${({ theme }) => theme.palette.black}; -`; - -const EmptyChartText = styled.div` - color: ${({ theme }) => theme.palette.brand}; - font-size: 16px; - line-height: 19px; -`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/ChartWrapper.tsx` around lines 233 - 247, EmptyChartContainer and EmptyChartText styled-components are defined but unused; remove these dead declarations (EmptyChartContainer and EmptyChartText) from ChartWrapper.tsx to clean up unused code, or alternatively reintroduce their usage where the TotalTGLDBid chart block is intended to render; update imports if any become unused after removal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsx`:
- Around line 72-81: The current mapping coerces malformed X values to 0 which
skews the domain; update the dataIndices and tickIndices logic to only include
finite numeric X values: when building dataIndices from chartData use
Number(value) (where value is d[xDataKey]) and filter with Number.isFinite to
drop non-finite/malformed entries, and for tickIndices map xTicks to Number and
also filter with Number.isFinite (instead of !isNaN); keep the existing
allIndices, minBucketIndex and maxBucketIndex fallback behavior when allIndices
is empty.
---
Nitpick comments:
In
`@apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/ChartWrapper.tsx`:
- Around line 233-247: EmptyChartContainer and EmptyChartText styled-components
are defined but unused; remove these dead declarations (EmptyChartContainer and
EmptyChartText) from ChartWrapper.tsx to clean up unused code, or alternatively
reintroduce their usage where the TotalTGLDBid chart block is intended to
render; update imports if any become unused after removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efbcd3e9-316c-4cf6-b54f-c347c9ef0e3f
📒 Files selected for processing (22)
apps/dapp/src/components/InputSelect/InputSelect.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/BidHistoryTable/Table.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/BarChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/TgldAuctionChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/hooks/use-bids-history.tsapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/index.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/Chart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/ChartWrapper.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Details/Details.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-closing-price-history.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/BarChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/DotChart.tsxapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/bidAggregation.tsapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/index.tsapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/types.tsapps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/yAxisUtils.tsapps/dapp/src/constants/newenv/test.tsapps/dapp/src/utils/subgraph.ts
💤 Files with no reviewable changes (3)
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/Chart.tsx
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/Chart2.tsx
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/Chart/BarChart.tsx
✅ Files skipped from review due to trivial changes (3)
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-closing-price-history.tsx
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/index.ts
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/types.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Bid/BidHistoryTable/Table.tsx
- apps/dapp/src/components/InputSelect/InputSelect.tsx
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/yAxisUtils.ts
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/components/Charts/BarChart.tsx
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/BidHistoryChart.tsx
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Chart/SpiceFinalPriceChart.tsx
- apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/hooks/use-bid-history.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
What does this PR solve?
We are introducing a new chart for both spice auctions and TGLD auctions.
Spice auctions:
TGLD Auctions:
Along with misc changes and fixes.
Fixes # (issue)
Checklist
Summary by CodeRabbit
New Features
Improvements
Chores