Update canvas renderer layout and audit docs #21
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| for (const id of Array.from(state.layout.keys())) { | ||
| if (state.seenGen.get(id) !== gen) { | ||
| removeAgent(state, id); | ||
| } |
There was a problem hiding this comment.
🟡 Memory leak: seenGen map entries never deleted when agents are removed
The seenGen map accumulates entries for all agents that have ever been seen but is never cleaned up when agents are removed.
Click to expand
How the bug occurs
In updateLayout, when an agent disappears from the agents array, removeAgent(state, id) is called at line 417. However, removeAgent (lines 354-368) only cleans up layout, agentGroupKey, and spatial - it never calls state.seenGen.delete(id).
The seenGen map is only cleared when agents.length === 0 (line 393), but in normal operation with agents coming and going, entries accumulate indefinitely.
Impact
In long-running applications where agents frequently start and stop, the seenGen map will grow unbounded, causing a memory leak. Each removed agent leaves behind an orphaned (id, generation) entry.
Expected behavior
When an agent is removed, its entry in seenGen should also be deleted to free memory.
Recommendation: Add state.seenGen.delete(id) inside the removeAgent function or in the removal loop at lines 415-418.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a447b89f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function findPlacement( | ||
| group: GroupState, | ||
| spatial: SpatialIndex, | ||
| maxAttempts = 256 | ||
| ): Coordinate | null { | ||
| while (group.freeStack.length) { | ||
| const key = group.freeStack.pop(); | ||
| if (key === undefined) break; | ||
| const { cx, cy } = unpackCell(key); | ||
| const coord = cellToWorld(cx, cy); | ||
| const bounds = boundsForCoord(coord); | ||
| if (!hasCollision(bounds, spatial)) return coord; | ||
| } | ||
|
|
||
| for (let i = 0; i < maxAttempts; i += 1) { | ||
| const { cx, cy } = spiralNext(group.spiral); | ||
| const coord = cellToWorld(cx, cy); | ||
| const bounds = boundsForCoord(coord); | ||
| if (!hasCollision(bounds, spatial)) return coord; | ||
| } |
There was a problem hiding this comment.
findPlacement now hard-stops after 256 spiral steps. In dense layouts or any group with more than ~256 occupied positions around the anchor, findPlacement returns null, and addAgent silently skips placement, so new agents never appear on the canvas. The prior logic expanded the search radius with layout size, so this is a regression for larger/denser groups. Consider making the attempt budget scale with group size or continuing until a free cell is found (or logging/fallback) to avoid missing agents.
Useful? React with 👍 / 👎.
1 participant
Summary
Testing
Ensure you read the contribution guide before submitting a commit and follow the exact guidelines.