bito-code-review[bot] commented on code in PR #41132:
URL: https://github.com/apache/superset/pull/41132#discussion_r3488798767
##########
superset-frontend/src/features/allEntities/AllEntitiesTable.tsx:
##########
@@ -59,7 +59,7 @@ interface AllEntitiesTableProps {
}
export default function AllEntitiesTable({
- search = '',
+ search: _search = '',
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Unused prop dead code</b></div>
<div id="fix">
The `search` prop is destructured as `_search` but never used in the
function body (lines 66-174). The caller at `AllEntities/index.tsx:241` passes
`search={tag?.name || ''}`, but this value has no effect. Either remove the
unused prop from the interface and destructuring, or wire it up to a functional
filter. An underscore prefix in TypeScript does not eliminate dead code — it
only signals intentionality.
</div>
</div>
<small><i>Code Review Run #7c4a0e</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/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx:
##########
@@ -118,7 +118,6 @@ interface MatrixifyGridRendererProps {
function MatrixifyGridRenderer({
formData,
datasource,
- width,
height,
hooks,
}: MatrixifyGridRendererProps) {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Stale interface property</b></div>
<div id="fix">
The `MatrixifyGridRendererProps` interface still declares `width?: number`
at line 113, but the diff removes `width` from the destructured props.
`SuperChart.tsx` (line 223) still passes `width={chartWidth}`, so the prop is
available but unused. Remove the interface entry to prevent API contract
mismatch.
</div>
</div>
<small><i>Code Review Run #7c4a0e</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/dashboard/components/gridComponents/TabsRenderer/TabsRenderer.tsx:
##########
@@ -170,7 +169,6 @@ const TabsRenderer = memo<TabsRendererProps>(
tabBarPaddingLeft = 0,
onTabsReorder,
isEditingTabTitle = false,
- onTabTitleEditingChange,
}) => {
const [activeId, setActiveId] = useState<string | null>(null);
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Unwired prop causes regression</b></div>
<div id="fix">
The `onTabTitleEditingChange` prop is defined in `TabsRendererProps` (line
111) and passed from parent `Tabs.tsx` (line 547), but the destructuring at
line 173 was removed. This breaks the callback contract — callers expecting to
receive editing state changes will never be notified. The prop flows through
`isEditingTabTitle` to disable drag during title editing, but the callback to
notify parent of state changes is silently dropped.
</div>
</div>
<small><i>Code Review Run #7c4a0e</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/components/DataTablesPane/components/SamplesPane.tsx:
##########
@@ -60,7 +60,6 @@ export const SamplesPane = ({
queryFormData,
queryForce,
setForceQuery,
- isVisible,
canDownload,
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete prop removal inconsistency</b></div>
<div id="fix">
Incomplete removal: `isVisible` is removed from component's destructured
props at line 63, but the prop is still defined in `SamplesPaneProps`
(types.ts:62) and still being passed by caller `DataTablesPane.tsx:213`. This
creates a type contract mismatch where callers pass a prop the component no
longer accepts. Either remove `isVisible` from both the type definition and all
callers, or keep it if intended for future use.
</div>
</div>
<small><i>Code Review Run #7c4a0e</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/dashboard/components/nativeFilters/FilterBar/Vertical.tsx:
##########
@@ -141,8 +141,6 @@ const VerticalFilterBar: FC<VerticalBarProps> = ({
onPendingCustomizationDataMaskChange,
toggleFiltersBar,
width,
- clearAllTriggers,
- onClearAllComplete,
}) => {
const theme = useTheme();
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Props removed from child but passed by parent</b></div>
<div id="fix">
The diff removes `clearAllTriggers` and `onClearAllComplete` from
Vertical.tsx props destructuring, but the parent component `index.tsx` at lines
674-675 still passes these props to `<Vertical />`. This creates an interface
contract violation where the parent passes props the child no longer accepts.
Either remove the prop passes in index.tsx or revert the removals in
Vertical.tsx.
</div>
</div>
<small><i>Code Review Run #7c4a0e</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]
