Refactor snapshot deltas and activity checks #12
Conversation
![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: 3ffb533fa0
ℹ️ 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".
| const groupKey = state.agentGroupKey.get(id); | ||
| if (groupKey && coord) { | ||
| const group = state.groups.get(groupKey); | ||
| if (group) { | ||
| group.freeStack.push(gridKeyFromWorld(coord)); | ||
| } | ||
| } | ||
| state.agentGroupKey.delete(id); |
There was a problem hiding this comment.
removeAgent only drops the layout entry and agentGroupKey but never clears seenGen or groups/groupAnchors for identities that disappear. In a long‑running server with many short‑lived sessions or changing group keys, these maps will grow without bound, increasing memory use and slowing layout maintenance. Consider deleting the removed id from seenGen and removing group state/anchors when a group has no remaining agents (the previous implementation pruned inactive group anchors each tick).
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| function removeAgent(state: LayoutState, id: string): void { | ||
| const coord = state.layout.get(id); | ||
| if (coord) { | ||
| unindexAgent(id, state.spatial); | ||
| } | ||
| state.layout.delete(id); | ||
|
|
||
| const groupKey = state.agentGroupKey.get(id); | ||
| if (groupKey && coord) { | ||
| const group = state.groups.get(groupKey); | ||
| if (group) { | ||
| group.freeStack.push(gridKeyFromWorld(coord)); | ||
| } | ||
| } | ||
| state.agentGroupKey.delete(id); | ||
| } |
There was a problem hiding this comment.
🟡 Memory leak: seenGen map entries not cleaned up when agents are removed
The removeAgent function in layout.ts doesn't delete the agent's entry from the seenGen map, causing unbounded memory growth over time.
Click to expand
Issue
When an agent is removed via removeAgent() at line 354-368, the function cleans up layout, spatial, and agentGroupKey, but never removes the agent's entry from seenGen.
Code flow
updateLayout()adds agents toseenGenat line 404:state.seenGen.set(id, gen);- When agents disappear,
removeAgent()is called at line 417 removeAgent()cleans up other maps but notseenGenseenGenis only cleared whenagents.length === 0(line 393) orsetLayoutPositions(line 457)
Impact
Over time, as agents come and go, seenGen accumulates stale entries. In long-running sessions with many agent churn, this causes unbounded memory growth.
Expected behavior
removeAgent() should include state.seenGen.delete(id);
Recommendation: Add state.seenGen.delete(id); to the removeAgent function after line 368.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Superseded by cherry-picked Codex tail fixes in fix/activity-state. |
1 participant
Summary
Performance impact
Tradeoffs
Testing