Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![xgopilot[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
| if hasToolResultPart(msg) { | ||
| continue | ||
| } | ||
| if hasNonEmptyTextPart(msg) { |
There was a problem hiding this comment.
userEffectiveInputCount only increments for non-empty text parts. User turns that contain non-text input only (for example image/file parts) are counted as zero, so IsZeroMessageSession can misclassify real sessions and let PurgeZeroMessageSessions delete them. Consider counting any meaningful non-tool-result user content, not just text.
| if m.sess != nil { | ||
| excludeID = m.sess.ID | ||
| } | ||
| _, _, _ = m.store.PurgeZeroMessageSessions(m.workspace, excludeID) |
There was a problem hiding this comment.
loadSessionsCmd now runs purge immediately before List, and defaultSessionLimit is now unbounded (0). That means a full scan/decode pass for purge plus another full scan/decode for listing on each reload. With many sessions this will regress responsiveness. Consider either restoring a bounded list limit or combining cleanup and listing into one pass.
| "- Long pasted code/text is compressed to `[Paste #N ~X lines]`.", | ||
| "- Use `[Paste]`, `[Paste #N]`, `[Paste line3]`, or `[Paste #N line3~line7]` to expand references.", | ||
| "- Restore in TUI: press `Ctrl+L` or run `/session`, choose a session, then press `Enter`.", | ||
| "- CLI-only restore: use `/resume <id>` in `bytemind chat`.", |
There was a problem hiding this comment.
This help line says to use /resume <id> in bytemind chat, but in this PR /resume is CLI-only while TUI restore flow is /session + Enter. The wording is currently contradictory and will mislead users.
| if hasToolResultPart(msg) { | ||
| continue | ||
| } | ||
| if hasNonEmptyTextPart(msg) { |
There was a problem hiding this comment.
userEffectiveInputCount only increments when a user message has non-empty text parts. User turns with non-text input only (for example file/image parts) are treated as zero-input, so IsZeroMessageSession can incorrectly classify real sessions as disposable and purge them. Consider counting any meaningful non-tool-result user content, not just text parts.
| if m.sess != nil { | ||
| excludeID = m.sess.ID | ||
| } | ||
| _, _, _ = m.store.PurgeZeroMessageSessions(m.workspace, excludeID) |
There was a problem hiding this comment.
loadSessionsCmd now purges on every load and then calls List with defaultSessionLimit = 0 (unbounded). This causes two full directory scans/decodes per reload, which can degrade responsiveness as session count grows. Consider restoring a bounded list limit or combining purge + list into a single pass.
| if hasToolResultPart(msg) { | ||
| continue | ||
| } | ||
| if hasNonEmptyTextPart(msg) { |
There was a problem hiding this comment.
userEffectiveInputCount increments only when a user message has non-empty text parts. User turns containing only non-text input (e.g. image_ref) are treated as zero input, so IsZeroMessageSession can misclassify real sessions and PurgeZeroMessageSessions may delete them. Consider counting any meaningful non-tool-result user part, not only text.
| if m.sess != nil { | ||
| excludeID = m.sess.ID | ||
| } | ||
| _, _, _ = m.store.PurgeZeroMessageSessions(m.workspace, excludeID) |
There was a problem hiding this comment.
loadSessionsCmd now performs PurgeZeroMessageSessions before List, and defaultSessionLimit is set to 0 (unbounded). This creates two full scans/loads on every sessions refresh, which will noticeably slow the TUI with large session stores. Consider restoring a bounded default limit or combining purge+list in a single pass.
| if hasToolResultPart(msg) { | ||
| continue | ||
| } | ||
| if hasNonEmptyTextPart(msg) { |
There was a problem hiding this comment.
userEffectiveInputCount only increments for non-empty text parts. User turns can be image-only (image_ref parts are valid and produced by the TUI), so these sessions are misclassified as zero-message and may be purged. Count meaningful non-tool-result user content beyond text.
| if m.sess != nil { | ||
| excludeID = m.sess.ID | ||
| } | ||
| _, _, _ = m.store.PurgeZeroMessageSessions(m.workspace, excludeID) |
There was a problem hiding this comment.
loadSessionsCmd now purges immediately before List, while defaultSessionLimit is unbounded (0). This creates two full store passes on each reload (purge + list) and can noticeably degrade responsiveness with many sessions. Consider a bounded list limit or a single-pass cleanup/listing path.
| if hasToolResultPart(msg) { | ||
| continue | ||
| } | ||
| if hasNonEmptyTextPart(msg) { |
There was a problem hiding this comment.
userEffectiveInputCount only treats non-empty text as user input. User turns that are valid but non-text (for example image_ref) will be counted as zero, so IsZeroMessageSession can incorrectly classify and purge real sessions. Consider counting any non-tool-result user content, not just text parts.
| if m.sess != nil { | ||
| excludeID = m.sess.ID | ||
| } | ||
| _, _, _ = m.store.PurgeZeroMessageSessions(m.workspace, excludeID) |
There was a problem hiding this comment.
loadSessionsCmd now does PurgeZeroMessageSessions immediately before List, and defaultSessionLimit is 0 (unbounded). This creates two full-store scans/loads on every refresh and can degrade responsiveness as session count grows. Consider a bounded default or a single-pass strategy that combines cleanup + listing.
1 participant
No description provided.