codeant-ai-for-open-source[bot] commented on code in PR #38094:
URL: https://github.com/apache/superset/pull/38094#discussion_r2895274830
##########
superset-frontend/src/visualizations/TimeTable/utils/numberUtils/numberUtils.ts:
##########
@@ -22,10 +22,11 @@
* @param value - The value to parse (string, number, or null)
Review Comment:
**Suggestion:** The JSDoc still documents this helper as returning 0 for
invalid inputs, but the implementation and tests now return null, which can
mislead future callers into assuming they don't need to handle null and cause
downstream logic errors; the comment should be updated to reflect the actual
behavior. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Misleading docs for parseToNumber in TimeTable utilities.
- ⚠️ Future callers may omit null checks on invalid input.
```
</details>
```suggestion
* @param value - The value to parse (string, number, null, or undefined)
* @returns The numeric value or null if invalid or not parseable
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open
`superset-frontend/src/visualizations/TimeTable/utils/numberUtils/numberUtils.ts`
and inspect the JSDoc for `parseToNumber` at lines 20–24, which states
`@returns The
numeric value or 0 if invalid`.
2. In a TypeScript/JavaScript environment within this project, import
`parseToNumber` from
`superset-frontend/src/visualizations/TimeTable/utils/numberUtils/numberUtils.ts`.
3. Call `parseToNumber('not-a-number')` or `parseToNumber(undefined)` and
observe the
returned value.
4. Verify that the implementation at lines 25–31 returns `null` for these
invalid inputs
(due to `Number.isNaN(numericValue) ? null : numericValue`), contradicting
the documented
return of `0`; this mismatch can mislead any current or future caller
relying on the
JSDoc.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/visualizations/TimeTable/utils/numberUtils/numberUtils.ts
**Line:** 22:22
**Comment:**
*Logic Error: The JSDoc still documents this helper as returning 0 for
invalid inputs, but the implementation and tests now return null, which can
mislead future callers into assuming they don't need to handle null and cause
downstream logic errors; the comment should be updated to reflect the actual
behavior.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38094&comment_hash=2b6f9d04382d414cc176d47a77b0bcf16c3133dea52ca2aba49fec38c2fb0e0c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38094&comment_hash=2b6f9d04382d414cc176d47a77b0bcf16c3133dea52ca2aba49fec38c2fb0e0c&reaction=dislike'>👎</a>
##########
superset-frontend/src/visualizations/TimeTable/TimeTable.tsx:
##########
@@ -91,18 +97,26 @@ const TimeTable = ({
entries={entries}
/>
),
+ cellValues: {
+ ...(acc.cellValues as object),
+ [columnConfig.key]: value
+ }
};
}
return {
...acc,
[columnConfig.key]: (
<ValueCell
- valueField={valueField}
+ value={value}
column={columnConfig}
- reversedEntries={reversedEntries}
+ errorMsg={errorMsg}
/>
),
+ cellValues: {
+ ...(acc.cellValues as object),
+ [columnConfig.key]: value
+ }
};
Review Comment:
**Suggestion:** The reducer that builds `cellValues` spreads
`acc.cellValues` directly, which is `undefined` on the first iteration;
spreading `undefined` in an object literal throws a TypeError at runtime, so
the TimeTable will crash the first time rows are processed. Wrap
`acc.cellValues` in a fallback object so that an empty object is used when it
hasn't been initialized yet. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Time-series Table visualization crashes when rendering any data.
- ❌ Column sorting and cell rendering never execute due to crash.
```
</details>
```suggestion
if (columnConfig.colType === 'spark') {
return {
...acc,
[columnConfig.key]: (
<Sparkline
valueField={valueField}
column={columnConfig}
entries={entries}
/>
),
cellValues: {
...((acc.cellValues as object) || {}),
[columnConfig.key]: value,
},
};
}
return {
...acc,
[columnConfig.key]: (
<ValueCell
value={value}
column={columnConfig}
errorMsg={errorMsg}
/>
),
cellValues: {
...((acc.cellValues as object) || {}),
[columnConfig.key]: value,
},
};
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open any Time-series Table chart in the Superset UI, which renders the
`TimeTable`
React component from
`superset-frontend/src/visualizations/TimeTable/TimeTable.tsx`.
2. `TimeTable` renders and executes the `useMemo` block that builds
`memoizedRows` (around
lines 74–127), calling `columnConfigs.reduce` with an initial accumulator
`{}` for each
row.
3. On the first `reduce` iteration for a given row, `acc` is `{}`, so
`acc.cellValues` is
`undefined` when the reducer executes the code at lines 90–120 that builds
`cellValues: {
...(acc.cellValues as object), [columnConfig.key]: value }`.
4. At runtime, spreading `undefined` in an object literal (`{ ...(undefined
as object) }`)
throws a `TypeError: Cannot convert undefined or null to object`, causing
the `TimeTable`
component to fail rendering and the Time-series Table chart to crash
whenever it is
opened.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/visualizations/TimeTable/TimeTable.tsx
**Line:** 90:120
**Comment:**
*Logic Error: The reducer that builds `cellValues` spreads
`acc.cellValues` directly, which is `undefined` on the first iteration;
spreading `undefined` in an object literal throws a TypeError at runtime, so
the TimeTable will crash the first time rows are processed. Wrap
`acc.cellValues` in a fallback object so that an empty object is used when it
hasn't been initialized yet.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38094&comment_hash=6eb947a10764a7c4a92a2c18db90ec4b0c5a63df08d0681ae93618dc2e5293e8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38094&comment_hash=6eb947a10764a7c4a92a2c18db90ec4b0c5a63df08d0681ae93618dc2e5293e8&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
