bito-code-review[bot] commented on code in PR #36760:
URL: https://github.com/apache/superset/pull/36760#discussion_r2637412665
##########
superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.test.ts:
##########
@@ -232,7 +232,8 @@ test('returns column keywords among selected tables', async
() => {
);
storeWithSqlLab.dispatch(
addTable(
- { id: expectQueryEditorId },
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ { id: expectQueryEditorId } as any,
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect type assertion in test</b></div>
<div id="fix">
The test is passing { id: expectQueryEditorId } as any to addTable, but
addTable expects a full QueryEditor object. This bypasses TypeScript checks and
may lead to runtime issues since getUpToDateQuery will fail to find the
queryEditor in state. Consider setting up the store with a proper QueryEditor
or using an existing one.
</div>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/90f99b5/.cursor/rules/dev-standard.mdc#L16";>dev-standard.mdc:16</a>
</li>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/90f99b5/AGENTS.md#L57";>AGENTS.md:57</a>
</li>
</ul>
</details>
<small><i>Code Review Run #4f0eeb</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/SqlLab/reducers/sqlLab.ts:
##########
@@ -753,7 +780,8 @@ export default function sqlLabReducer(state = {}, action) {
}
return true;
})
- .map(([id, query]) => [
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ .map(([id, query]: [string, any]) => [
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Type Safety Violation</b></div>
<div id="fix">
The type annotation uses 'any' for the query parameter, which violates the
project's strict TypeScript guidelines against using 'any' types. Since the
query is of type QueryResponse & { inLocalStorage?: boolean }, use that instead
for better type safety.
</div>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/90f99b5/.cursor/rules/dev-standard.mdc#L16";>dev-standard.mdc:16</a>
</li>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/90f99b5/AGENTS.md#L57";>AGENTS.md:57</a>
</li>
</ul>
</details>
<small><i>Code Review Run #4f0eeb</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.test.ts:
##########
@@ -276,7 +277,8 @@ test('returns column keywords among selected tables', async
() => {
act(() => {
storeWithSqlLab.dispatch(
addTable(
- { id: expectQueryEditorId },
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ { id: expectQueryEditorId } as any,
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect type assertion in test</b></div>
<div id="fix">
Similar to the first instance, this test call passes { id:
expectQueryEditorId } as any to addTable, bypassing type checks. The function
expects a complete QueryEditor, and this may cause getUpToDateQuery to return
undefined dbId, breaking the table creation logic.
</div>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/90f99b5/.cursor/rules/dev-standard.mdc#L16";>dev-standard.mdc:16</a>
</li>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/90f99b5/AGENTS.md#L57";>AGENTS.md:57</a>
</li>
</ul>
</details>
<small><i>Code Review Run #4f0eeb</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/controlUtils/standardizedFormData.ts:
##########
@@ -189,11 +188,10 @@ export class StandardizedFormData {
transform(
targetVizType: string,
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
exploreState: Record<string, any>,
- ): {
- formData: QueryFormData;
- controlsState: ControlStateMapping;
- } {
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ ): any {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Type Safety Violation</b></div>
<div id="fix">
The return type change from a structured object to 'any' reduces TypeScript
type safety and violates ongoing frontend modernization standards that prohibit
'any' types. While this may resolve CI errors, it risks runtime issues if
callers assume the object structure. Consider defining a proper interface for
the return value to align with the 'NO any types' rule in dev standards.
</div>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/90f99b5/.cursor/rules/dev-standard.mdc#L16";>dev-standard.mdc:16</a>
</li>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/90f99b5/AGENTS.md#L57";>AGENTS.md:57</a>
</li>
</ul>
</details>
<small><i>Code Review Run #4f0eeb</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
