fix(memory): surface honest notice for bodyless session reads#310
Merged
Conversation
A session `cat` could return 0 bytes with no signal while `ls` showed a non-zero size. A captured event is never legitimately empty (capture.ts always writes a populated JSON entry), so a session that reconstructs to "" is always a pathology — yet both readers returned it as an empty file, and `ls` size comes from a separate MAX(size_bytes) query so it stayed non-zero, producing the misleading size-vs-content split. - grep-core: add shared emptySessionBodyNotice(rowCount). - virtual-table-query (hook path): count session rows even when message::text is NULL (previously dropped silently), skip empty normalized parts, and emit the notice when rows exist but no readable body — instead of "" (silent empty) or null (misleading "No such file" though ls lists it). - deeplake-fs (shell path): normalizeSessionMessage coerces null/undefined/ non-serializable to "" (no more literal "null"/"undefined" leaking); joinSessionMessages drops empty parts; prefetch/readFile/readFileBuffer emit the notice on empty reconstruction. Tests: NULL/undefined/empty-string bodies, no-leak guarantee, and mixed empty+real turns (keeps only real turns) across both reader paths.
📝 WalkthroughWalkthroughSession-backed virtual and filesystem reads now distinguish stored rows with empty or null bodies from missing files, filter unusable message parts, and return a row-count-aware empty-body notice. Tests cover empty, nullish, and mixed session messages. ChangesSession body handling
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 3 files changed
Generated for commit c6a003e. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A session
catthrough the memory mirror could return 0 bytes with no signal whilelsshowed a non-zero size — making the mirror look populated when the transcript body was actually unreadable, and blocking fallback to raw session data for turn-level detail.Root cause, both plugin-side (backend unchanged):
capture.tsalways writes a populated JSON entry), so a session that reconstructs to""is always a pathology — but both readers returned it as an empty file.lssize comes from a separateMAX(size_bytes)query, so it stayed non-zero → the misleading size-vs-content split.normalizeSessionMessagedidJSON.stringify(message)unconditionally: aNULLcolumn became the literal text"null", andundefinedleaked into the joined output.Fix:
grep-core.ts— sharedemptySessionBodyNotice(rowCount): honest diagnostic ("rows exist, body empty, metadata only — not an empty file").virtual-table-query.ts(hook path — what Claude Code's bashcathits) — count session rows even whenmessage::textis NULL (previously dropped silently), skip empty normalized parts, emit the notice instead of""(silent empty) ornull(misleading "No such file" thoughlslists it).deeplake-fs.ts(shell path) —normalizeSessionMessagecoerces null/undefined/non-serializable →"";joinSessionMessagesfilters empty parts;prefetch/readFile/readFileBufferemit the notice on empty reconstruction.Scope note: this makes the empty-read case honest, not silent — it does not change why a body would be empty (a separate capture/storage question, answerable per-row via
length(message::text)vssize_bytes).Version Bump
Bug fix → warrants a patch bump (
0.7.126→0.7.127). Not bumped yet —package.jsonis unchanged, so as-is this merges with no release. Will bump on request to ship it.Test plan
npx vitest runon affected suites: 376 passed — deeplake-fs, virtual-table-query, grep-core, pre-tool-use)"null"/"undefined"-leak guarantee, mixed empty+real turns across both reader paths)package.json, or no release needed for this change — pending decision (see Version Bump)🤖 Generated with Claude Code