fix: add safety check to fileTree memoization - #537
Conversation
Closes piyushdotcomm#527 collectFilePaths was recently wrapped in useMemo to prevent per-render traversal. This commit adds a safety check to ensure templateData.items exists before calling the function, preventing potential crashes if the template object structure is incomplete.
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
👋 Thanks for opening a PR, @Somil450!Your PR has entered the 🚦 PR Review Pipeline.
What happens next
A pipeline status comment will appear below and update automatically as your PR progresses. While you wait
This comment is posted only once. |
PR Summary by QodoFix fileTree memoization crash when templateData.items is missing
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
22 rules 1. Items not array-guarded
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| // Memoize the file tree string to avoid re-computing on every render | ||
| const fileTree = useMemo( | ||
| () => templateData ? collectFilePaths(templateData.items).join("\n") : "", | ||
| () => templateData?.items ? collectFilePaths(templateData.items).join("\n") : "", |
There was a problem hiding this comment.
1. Ai-chat-panel.tsx over 500 lines 📘 Rule violation ⚙ Maintainability
modules/playground/components/ai-chat-panel.tsx is 510 lines long, exceeding the 500-line maximum. This increases maintenance overhead and makes the component harder to review and safely evolve.
Agent Prompt
## Issue description
`modules/playground/components/ai-chat-panel.tsx` exceeds the 500-line limit (currently 510 lines), violating the project file-size compliance requirement.
## Issue Context
This PR modifies the file, so it is in the changed-path set and must comply with the 500-line cap.
## Fix Focus Areas
- modules/playground/components/ai-chat-panel.tsx[1-510]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // Memoize the file tree string to avoid re-computing on every render | ||
| const fileTree = useMemo( | ||
| () => templateData ? collectFilePaths(templateData.items).join("\n") : "", | ||
| () => templateData?.items ? collectFilePaths(templateData.items).join("\n") : "", |
There was a problem hiding this comment.
2. Items not array-guarded 🐞 Bug ☼ Reliability
The new templateData?.items check only tests truthiness, so a truthy non-array items value can still be passed to collectFilePaths(...) and throw during the for...of traversal. AIChatPanel tool handlers also pass templateData.items into helpers that assume arrays, so malformed DB-parsed template data can still crash later during tool execution.
Agent Prompt
## Issue description
`AIChatPanel` now checks `templateData?.items` before building `fileTree`, but this is only a truthy check. If `templateData` is malformed (e.g., parsed from DB JSON without `items`, or with `items` not being an array), the chat panel can still crash:
- In the `fileTree` memoization, by calling `collectFilePaths` with a non-array.
- In tool handlers (edit/delete), by passing `templateData.items` into helpers that expect arrays.
## Issue Context
`getPlaygroundById` parses template JSON from the DB without validating that it matches the expected `TemplateFolder` schema (including `items: TemplateItem[]`). This makes runtime malformed data plausible even if TypeScript types say otherwise.
## Fix Focus Areas
- modules/playground/components/ai-chat-panel.tsx[83-87]
- modules/playground/components/ai-chat-panel.tsx[207-292]
- modules/playground/actions/index.ts[121-156]
- modules/playground/hooks/useAI.ts[145-157]
## Suggested fix
1. Normalize once in `AIChatPanel`:
- `const templateItems = Array.isArray(templateData?.items) ? templateData.items : [];`
2. Build `fileTree` from `templateItems`:
- `useMemo(() => collectFilePaths(templateItems).join("\n"), [templateItems])`
3. In tool handlers, use `templateItems` (or guard with `Array.isArray(templateData?.items)` and return a user-friendly error) before calling `addOrUpdateFile/deleteFileByPath/findFileByPath`.
4. (Optional but stronger) Validate/repair the parsed DB JSON in `getPlaygroundById` (e.g., if parsed object lacks `items`, set `items: []` or treat it as invalid and fall back to scanning).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2 participants
Summary
Closes #527
Adds a safety check to the
fileTreememoization block inai-chat-panel.tsxto ensuretemplateData.itemsis defined before traversing it.Context
The original issue called out that
collectFilePaths()was being run on every render. However, a recent refactor had actually already wrapped it inuseMemo.This PR "officially" closes the issue by making that existing memoization robust. The previous code checked if
templateDataexisted, but not if theitemsarray was populated. If the object structure was incomplete,collectFilePathswould crash the chat panel.Change