[Prototype] AI-based formula bar #3442
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## qa #3442 +/- ##
=======================================
Coverage 89.71% 89.71%
=======================================
Files 434 434
Lines 89063 89063
=======================================
Hits 79906 79906
Misses 9157 9157 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
|
|
||
| // Handle window resize to recalculate height if needed | ||
| const handleResize = () => { | ||
| setTimeout(adjustHeight, 0); // Use setTimeout to ensure DOM is updated |
There was a problem hiding this comment.
Timer leak: setTimeout(adjustHeight, 0) is called in the resize handler but the timeout ID is not stored or cleared. If the component unmounts while the timeout is pending, the timeout will still execute and call adjustHeight() on an unmounted component, potentially causing errors.
| setTimeout(adjustHeight, 0); // Use setTimeout to ensure DOM is updated | |
| const timeoutId = setTimeout(adjustHeight, 0); // Use setTimeout to ensure DOM is updated | |
| return () => clearTimeout(timeoutId); // Clean up timeout if component unmounts |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| requestAnimationFrame(() => { | ||
| adjustHeight(); | ||
| }); |
There was a problem hiding this comment.
Timer leak: requestAnimationFrame() is called but the frame ID is not stored or cleared. If the component unmounts while the animation frame is pending, it will still execute and call adjustHeight() on an unmounted component, potentially causing errors.
| requestAnimationFrame(() => { | |
| adjustHeight(); | |
| }); | |
| const rafId = requestAnimationFrame(() => { | |
| adjustHeight(); | |
| }); | |
| return () => { | |
| cancelAnimationFrame(rafId); | |
| }; |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| private summaries = new Map<string, CodeCellSummary>(); | ||
| private readonly STORAGE_KEY = 'quadratic_ai_code_cell_summaries'; | ||
| private readonly MAX_SUMMARIES = 1000; // Limit to prevent memory issues | ||
| private saveTimeoutId: NodeJS.Timeout | null = null; |
There was a problem hiding this comment.
Timer leak vulnerability: saveTimeoutId is declared as NodeJS.Timeout but in browser environments, setTimeout returns a number. This type mismatch can cause clearTimeout to fail silently, leading to timer leaks and potential memory issues. Change type to number | null for browser compatibility.
| private saveTimeoutId: NodeJS.Timeout | null = null; | |
| private saveTimeoutId: number | NodeJS.Timeout | null = null; |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| const { x, y } = cursor; | ||
|
|
||
| // First check if this cell has an AI summary | ||
| const aiSummary = aiCodeCellSummaryStore.getSummary(sheets.current, x, y); |
There was a problem hiding this comment.
Cache invalidation bug: getSummary() is called without the currentCodeString parameter, so cache validation is skipped. This means outdated AI summaries will be displayed even when the code has changed. Pass the current code string to enable proper cache validation.
| const aiSummary = aiCodeCellSummaryStore.getSummary(sheets.current, x, y); | |
| const aiSummary = aiCodeCellSummaryStore.getSummary(sheets.current, x, y, currentCodeString); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
1 participant
This PR is purely for prototyping/experimentation.