codeant-ai-for-open-source[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2913232545
##########
superset-frontend/src/SqlLab/components/QueryTable/index.tsx:
##########
@@ -52,10 +52,10 @@ interface QueryTableQuery extends Omit<
QueryResponse,
'state' | 'sql' | 'progress' | 'results' | 'duration' | 'started'
> {
- state?: Record<string, any>;
- sql?: Record<string, any>;
- progress?: Record<string, any>;
- results?: Record<string, any>;
+ state?: ReactNode;
+ sql?: ReactNode;
+ progress?: ReactNode;
+ results?: ReactNode;
duration?: ReactNode;
started?: ReactNode;
Review Comment:
**Suggestion:** The `QueryTableQuery` type omits and redefines several
fields from `QueryResponse` to be `ReactNode`, but `user` and `db` are not
omitted and thus retain their original `Record<string, any>` types while later
being assigned JSX elements, creating a type mismatch that breaks the contract
of `QueryTableQuery` and can cause TypeScript errors or misuse of these fields
as non-object values elsewhere. [type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ SqlLab QueryTable file fails TypeScript type-checking.
- ❌ Frontend build/tests can fail, blocking deployments.
- ⚠️ Query history UI cannot be shipped with current types.
```
</details>
```suggestion
'state' | 'sql' | 'progress' | 'results' | 'duration' | 'started' | 'user'
| 'db'
> {
state?: ReactNode;
sql?: ReactNode;
progress?: ReactNode;
results?: ReactNode;
duration?: ReactNode;
started?: ReactNode;
user?: ReactNode;
db?: ReactNode;
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `superset-frontend/src/SqlLab/components/QueryTable/index.tsx` and
locate
`QueryTableQuery` at lines 51–61, which extends `QueryResponse` but only
omits `'state' |
'sql' | 'progress' | 'results' | 'duration' | 'started'`.
2. In the same file, inspect the `data` `useMemo` mapping at lines 250–279
where `const q
= rest as QueryTableQuery;` is created and `q.user` and `q.db` are assigned
JSX `<Button>`
elements (lines 262–279).
3. Inspect `superset-frontend/src/SqlLab/fixtures.ts` where `baseQuery:
QueryResponse` is
defined at lines 587–672 and note that `db` and `user` are initialized as
objects: `db: {
key: 'main' }` and `user: { key: 'admin' }` at lines 611–612, evidencing that
`QueryResponse['db']` and `QueryResponse['user']` are object-shaped, not
ReactNode.
4. Run the TypeScript type checker or frontend build (e.g., `npm test` or
`npm run build`)
in this PR: the compiler will type-check `QueryTable/index.tsx`, see
`q.user` and `q.db`
being assigned React elements where their inherited types from
`QueryResponse` are object
types, and emit errors similar to "Type 'ReactElement<...>' is not
assignable to type
'Record<string, any>'" for these assignments, causing the SqlLab frontend to
fail
compilation.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/SqlLab/components/QueryTable/index.tsx
**Line:** 53:60
**Comment:**
*Type Error: The `QueryTableQuery` type omits and redefines several
fields from `QueryResponse` to be `ReactNode`, but `user` and `db` are not
omitted and thus retain their original `Record<string, any>` types while later
being assigned JSX elements, creating a type mismatch that breaks the contract
of `QueryTableQuery` and can cause TypeScript errors or misuse of these fields
as non-object values elsewhere.
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%2F38563&comment_hash=8d5f297e6c81f47c3ab3af656f9579f1c4660b1a65d2c57113b883a59b633056&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=8d5f297e6c81f47c3ab3af656f9579f1c4660b1a65d2c57113b883a59b633056&reaction=dislike'>👎</a>
##########
superset-frontend/src/components/Chart/Chart.tsx:
##########
@@ -235,16 +235,13 @@ class Chart extends PureComponent<ChartProps, {}> {
);
}
- handleRenderContainerFailure(
- error: Error,
- info: { componentStack: string } | null,
- ) {
+ handleRenderContainerFailure(error: Error, info: ErrorInfo) {
Review Comment:
**Suggestion:** The error handler logs chart render metrics using
`this.renderStartTime`, but this field is never initialized in the `Chart`
component, so `start_offset` will be `undefined` and `duration` will be `NaN`,
producing invalid telemetry data whenever the error boundary is triggered. Use
a safe fallback when `renderStartTime` is not a valid number so that logging
always uses numeric values. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Chart error telemetry has undefined start_offset and NaN duration.
- ⚠️ LOG_ACTIONS_RENDER_CHART analytics receive corrupted timing data.
```
</details>
--
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]
