codeant-ai-for-open-source[bot] commented on code in PR #44235:
URL: https://github.com/apache/superset/pull/44235#discussion_r4003014529
##########
superset-frontend/src/dashboard/containers/DashboardComponent.tsx:
##########
@@ -156,9 +157,25 @@ const DashboardComponent = (
return {};
}, [component, dashboardLayout, props.id]);
- const Component = component
- ? componentLookup[component.type as keyof typeof componentLookup]
- : null;
+ // A stored layout is not guaranteed to be a tree: it can name a component
+ // that is also one of its own ancestors. Every nested component renders
+ // through this container, so rendering that repeat would recurse without
+ // end and the dashboard would never appear. Stop at the repeat instead, so
+ // the rest of the dashboard still renders and the layout can be repaired.
+ const isRepeatedAncestor =
+ props.id === props.parentId ||
+ Boolean(parentComponent?.parents?.includes(props.id));
Review Comment:
**Suggestion:** The repeat check runs after width calculation, so cyclic
rows recurse in `getDetailedComponentWidth` before this guard and still
overflow the stack during rendering. [logic error]
**Assessment:** ๐ด `Critical` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8f9bc71bdb7f487897d7ee15aaedac39&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8f9bc71bdb7f487897d7ee15aaedac39&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/containers/DashboardComponent.tsx
**Line:** 165:167
**Comment:**
*Logic Error: The repeat check runs after width calculation, so cyclic
rows recurse in `getDetailedComponentWidth` before this guard and still
overflow the stack during rendering.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44235&comment_hash=ecfd40596f9c1bc3b0e4cb630e18414eca4d1e3fed48196078ad54255efdf817&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44235&comment_hash=ecfd40596f9c1bc3b0e4cb630e18414eca4d1e3fed48196078ad54255efdf817&reaction=dislike'>๐</a>
##########
superset-frontend/src/dashboard/util/updateComponentParentsList.ts:
##########
@@ -44,7 +59,12 @@ export default function updateComponentParentsList({
if (Array.isArray(currentComponent.children)) {
currentComponent.children.forEach(childId => {
- if (layout[childId]) {
+ if (visited.has(childId)) {
+ logging.warn(
+ `The current layout reaches the component with the id:
${childId} more than once. Skipping this component`,
+ );
Review Comment:
**Suggestion:** Skipping later paths leaves a shared component's `parents`
set to its first path, so rendering it under another parent can use the wrong
tab and navigation ancestry. [api mismatch]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=32962b6425554169a4b0f7a957fdf795&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=32962b6425554169a4b0f7a957fdf795&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/util/updateComponentParentsList.ts
**Line:** 62:65
**Comment:**
*Api Mismatch: Skipping later paths leaves a shared component's
`parents` set to its first path, so rendering it under another parent can use
the wrong tab and navigation ancestry.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44235&comment_hash=d8831e3ee7de976d4114b5882c0e0794dc9d259a2f738fd5245c7d6345f0c6aa&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44235&comment_hash=d8831e3ee7de976d4114b5882c0e0794dc9d259a2f738fd5245c7d6345f0c6aa&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]
