|
| 1 | +# Implementation Plan: Session Analyzer JSONL Parser Fix (#3081) |
| 2 | + |
| 3 | +**Status**: Draft |
| 4 | +**Research Doc**: [research.md](./research.md) |
| 5 | +**Author**: opencode session |
| 6 | +**Date**: 2026-07-06 |
| 7 | +**Estimated Effort**: 2-3 hours |
| 8 | + |
| 9 | +## Overview |
| 10 | + |
| 11 | +### Summary |
| 12 | + |
| 13 | +Add a pre-filter to `terraphim_session_analyzer::parser::SessionParser::from_file()` that skips known Claude Code metadata JSONL entry types before attempting full deserialization. This eliminates ~2003 of ~2430 WARN lines without changing any downstream code. |
| 14 | + |
| 15 | +### Approach |
| 16 | + |
| 17 | +Pre-filter by entry type using a lightweight JSON peek. Only entry types that carry conversational message content (`user`, `assistant`) are deserialized into `SessionEntry`. Known metadata types are skipped silently at debug level. |
| 18 | + |
| 19 | +### Scope |
| 20 | + |
| 21 | +**In Scope:** |
| 22 | +- Pre-filter in `parser.rs::from_file()` |
| 23 | +- Skip set constant for known metadata types |
| 24 | +- Test fixtures for each metadata type |
| 25 | +- Test verifying zero WARN for metadata-only files |
| 26 | + |
| 27 | +**Out of Scope:** |
| 28 | +- Investigating the ~427 `assistant` parse failures (separate issue) |
| 29 | +- Changing `SessionEntry` struct fields |
| 30 | +- Modifying downstream consumers |
| 31 | +- The `terraphim_sessions` crate's own parser (different code) |
| 32 | + |
| 33 | +**Avoid At All Cost:** |
| 34 | +- Making `message` or `uuid` optional on `SessionEntry` (ripples through entire crate) |
| 35 | +- Adding new structs for metadata entry types (they carry no searchable content) |
| 36 | +- Adding abstraction layers or trait-based filtering (premature complexity) |
| 37 | + |
| 38 | +## Architecture |
| 39 | + |
| 40 | +### Data Flow (After Fix) |
| 41 | + |
| 42 | +``` |
| 43 | +Claude Code JSONL file |
| 44 | + | |
| 45 | + v |
| 46 | +parser.rs:from_file() -- reads line by line |
| 47 | + | |
| 48 | + v |
| 49 | +serde_json::from_str::<EntryTypePeek>(&line) <-- NEW: lightweight type extraction |
| 50 | + | |
| 51 | + +-- type in SKIP_SET? ---> debug!("Skipping metadata entry type: {ty}") |
| 52 | + | continue |
| 53 | + | |
| 54 | + +-- type not in SKIP_SET? |
| 55 | + | |
| 56 | + v |
| 57 | + serde_json::from_str::<SessionEntry>(&line) |
| 58 | + | | |
| 59 | + | Ok(entry) | Err(e) |
| 60 | + v v |
| 61 | + entries.push(entry) warn!("Failed to parse...") <-- still warns for genuine errors |
| 62 | +``` |
| 63 | + |
| 64 | +### Key Design Decisions |
| 65 | + |
| 66 | +| Decision | Rationale | Alternatives Rejected | |
| 67 | +|----------|-----------|----------------------| |
| 68 | +| Pre-filter by `"type"` field before full deserialize | Single-location change, zero downstream impact | Making fields Option<> (ripples through codebase) | |
| 69 | +| Use `EntryTypePeek` struct for lightweight parse | Cheaper than full `serde_json::Value` parse, type-safe | Regex matching on raw string (fragile) | |
| 70 | +| Skip at debug level, not silent | Traceability for debugging; can be turned on with RUST_LOG | Silent skip (loses all visibility) | |
| 71 | +| Constant `HashSet<&str>` for skip types | Compile-time known, O(1) lookup | Match statement (verbose for 12 types) | |
| 72 | + |
| 73 | +### Simplicity Check |
| 74 | + |
| 75 | +**What if this could be easy?** |
| 76 | + |
| 77 | +The simplest possible fix: a `const SKIP_TYPES: &[&str]` array and a `.contains()` check after a one-field JSON peek. That's 15 lines of code in one function. No struct changes. No new modules. No new dependencies. |
| 78 | + |
| 79 | +**Senior Engineer Test**: This is a filter clause, not an architecture change. Passes. |
| 80 | + |
| 81 | +**Nothing Speculative Checklist**: |
| 82 | +- [x] No features the user didn't request |
| 83 | +- [x] No abstractions "in case we need them later" |
| 84 | +- [x] No flexibility "just in case" |
| 85 | +- [x] No error handling for scenarios that cannot occur |
| 86 | +- [x] No premature optimization |
| 87 | + |
| 88 | +## File Changes |
| 89 | + |
| 90 | +### Modified Files |
| 91 | + |
| 92 | +| File | Changes | |
| 93 | +|------|---------| |
| 94 | +| `src/parser.rs` | Add `SKIP_ENTRY_TYPES` constant; add `EntryTypePeek` struct; add pre-filter in `from_file()` loop | |
| 95 | + |
| 96 | +### New Files |
| 97 | + |
| 98 | +| File | Purpose | |
| 99 | +|------|---------| |
| 100 | +| `tests/fixtures/metadata-entries.jsonl` | Test fixture: one line per metadata entry type | |
| 101 | +| `tests/skip_metadata.rs` | Integration test: zero WARN for metadata-only file | |
| 102 | + |
| 103 | +No new modules. No new dependencies. No struct changes. |
| 104 | + |
| 105 | +## API Design |
| 106 | + |
| 107 | +### New Types (internal, not exported) |
| 108 | + |
| 109 | +```rust |
| 110 | +/// Lightweight struct for peeking at the entry type before full deserialization. |
| 111 | +/// Only extracts the "type" field -- costs negligible overhead vs full SessionEntry parse. |
| 112 | +#[derive(Deserialize)] |
| 113 | +struct EntryTypePeek { |
| 114 | + #[serde(rename = "type")] |
| 115 | + entry_type: String, |
| 116 | +} |
| 117 | + |
| 118 | +/// Claude Code JSONL entry types that carry metadata, not conversational messages. |
| 119 | +/// These are skipped during parsing to avoid false deserialization failures. |
| 120 | +const SKIP_ENTRY_TYPES: &[&str] = &[ |
| 121 | + "last-prompt", |
| 122 | + "mode", |
| 123 | + "permission-mode", |
| 124 | + "ai-title", |
| 125 | + "file-history-snapshot", |
| 126 | + "queue-operation", |
| 127 | + "agent-name", |
| 128 | + "pr-link", |
| 129 | + "tool_reference", |
| 130 | + "text", |
| 131 | + "attachment", |
| 132 | + "system", |
| 133 | +]; |
| 134 | +``` |
| 135 | + |
| 136 | +No changes to public API. No new public types. No new error variants. |
| 137 | + |
| 138 | +## Test Strategy |
| 139 | + |
| 140 | +### Unit Tests |
| 141 | + |
| 142 | +| Test | Location | Purpose | |
| 143 | +|------|----------|---------| |
| 144 | +| `test_skip_metadata_entry_types` | `parser.rs` mod tests | Verify each metadata type is skipped without WARN | |
| 145 | +| `test_user_entry_still_parsed` | `parser.rs` mod tests | Verify `user` entries are still parsed | |
| 146 | +| `test_assistant_entry_still_parsed` | `parser.rs` mod tests | Verify `assistant` entries are still parsed | |
| 147 | +| `test_unknown_type_still_warns` | `parser.rs` mod tests | Verify genuinely unknown types still produce WARN | |
| 148 | +| `test_entry_type_peek_extracts_type` | `parser.rs` mod tests | Verify the peek struct works correctly | |
| 149 | + |
| 150 | +### Integration Tests |
| 151 | + |
| 152 | +| Test | Location | Purpose | |
| 153 | +|------|----------|---------| |
| 154 | +| `test_metadata_only_file_no_warnings` | `tests/skip_metadata.rs` | Full file of metadata entries produces zero entries and zero warnings | |
| 155 | + |
| 156 | +### Test Fixtures |
| 157 | + |
| 158 | +`tests/fixtures/metadata-entries.jsonl` -- one real JSONL line per metadata type, extracted from actual Claude Code session files: |
| 159 | + |
| 160 | +```jsonl |
| 161 | +{"type":"last-prompt","lastPrompt":"echo hello","leafUuid":"...","sessionId":"..."} |
| 162 | +{"type":"mode","mode":"normal","sessionId":"..."} |
| 163 | +{"type":"permission-mode","permissionMode":"auto","sessionId":"..."} |
| 164 | +{"type":"ai-title","title":"...","sessionId":"..."} |
| 165 | +{"type":"file-history-snapshot","...} |
| 166 | +{"type":"queue-operation","...} |
| 167 | +{"type":"agent-name","...} |
| 168 | +{"type":"pr-link","...} |
| 169 | +{"type":"tool_reference","...} |
| 170 | +{"type":"text","...} |
| 171 | +{"type":"attachment","...,"uuid":"..."} |
| 172 | +{"type":"system","...} |
| 173 | +``` |
| 174 | + |
| 175 | +## Implementation Steps |
| 176 | + |
| 177 | +### Step 1: Add skip set and peek struct |
| 178 | + |
| 179 | +**Files:** `src/parser.rs` (top of file, after imports) |
| 180 | +**Description:** Define `SKIP_ENTRY_TYPES` constant and `EntryTypePeek` struct |
| 181 | +**Tests:** `test_entry_type_peek_extracts_type` |
| 182 | +**Estimated:** 15 minutes |
| 183 | + |
| 184 | +### Step 2: Add pre-filter in from_file() |
| 185 | + |
| 186 | +**Files:** `src/parser.rs`, function `from_file()` at line 41-42 |
| 187 | +**Description:** Before `serde_json::from_str::<SessionEntry>`, peek at the type field. If in skip set, log at debug and continue. |
| 188 | +**Tests:** `test_skip_metadata_entry_types`, `test_user_entry_still_parsed`, `test_assistant_entry_still_parsed`, `test_unknown_type_still_warns` |
| 189 | +**Dependencies:** Step 1 |
| 190 | +**Estimated:** 30 minutes |
| 191 | + |
| 192 | +**Key code:** |
| 193 | + |
| 194 | +```rust |
| 195 | +// Inside the line-processing loop, before the SessionEntry deserialize: |
| 196 | +match serde_json::from_str::<EntryTypePeek>(&line) { |
| 197 | + Ok(peek) if SKIP_ENTRY_TYPES.contains(&peek.entry_type.as_str()) => { |
| 198 | + debug!("Skipping metadata entry of type: {}", peek.entry_type); |
| 199 | + continue; |
| 200 | + } |
| 201 | + Ok(_) => { /* fall through to full deserialize */ } |
| 202 | + Err(_) => { /* fall through -- will produce WARN as before */ } |
| 203 | +} |
| 204 | + |
| 205 | +// Existing deserialize logic unchanged: |
| 206 | +match serde_json::from_str::<SessionEntry>(&line) { |
| 207 | + Ok(entry) => { ... } |
| 208 | + Err(e) => { warn!(...); } |
| 209 | +} |
| 210 | +``` |
| 211 | + |
| 212 | +### Step 3: Create test fixtures |
| 213 | + |
| 214 | +**Files:** `tests/fixtures/metadata-entries.jsonl` |
| 215 | +**Description:** Extract one real JSONL line per metadata type from actual Claude Code session files |
| 216 | +**Tests:** Used by Step 4 integration test |
| 217 | +**Dependencies:** None |
| 218 | +**Estimated:** 20 minutes |
| 219 | + |
| 220 | +### Step 4: Integration test |
| 221 | + |
| 222 | +**Files:** `tests/skip_metadata.rs` |
| 223 | +**Description:** Parse the metadata-only fixture file and assert zero entries, zero warnings |
| 224 | +**Tests:** `test_metadata_only_file_no_warnings` |
| 225 | +**Dependencies:** Steps 1-3 |
| 226 | +**Estimated:** 20 minutes |
| 227 | + |
| 228 | +### Step 5: Verify with live data |
| 229 | + |
| 230 | +**Description:** Run `terraphim-agent sessions search "test"` and confirm WARN count drops from ~2430 to ~427 (only the `assistant` failures remain) |
| 231 | +**Dependencies:** Steps 1-4, cargo build + install |
| 232 | +**Estimated:** 15 minutes |
| 233 | + |
| 234 | +## Rollback Plan |
| 235 | + |
| 236 | +If issues discovered: |
| 237 | +1. Remove the `EntryTypePeek` struct and skip check -- the `from_file()` function reverts to direct deserialize |
| 238 | +2. No data migration needed -- the parser is stateless |
| 239 | + |
| 240 | +No feature flag needed -- the change is a strict improvement (fewer false warnings, same behaviour for valid entries). |
| 241 | + |
| 242 | +## Dependencies |
| 243 | + |
| 244 | +### New Dependencies |
| 245 | + |
| 246 | +None. Uses only `serde` and `serde_json` which are already dependencies. |
| 247 | + |
| 248 | +### Version Bump |
| 249 | + |
| 250 | +Publish as `terraphim-session-analyzer` `1.20.6` (patch bump from 1.20.3/1.20.5). |
| 251 | + |
| 252 | +Update consumer in `terraphim-clients/crates/terraphim_sessions/Cargo.toml`: |
| 253 | +```toml |
| 254 | +terraphim-session-analyzer = { version = "1.20.6", optional = true } |
| 255 | +``` |
| 256 | + |
| 257 | +## Crate Source Location |
| 258 | + |
| 259 | +The crate was extracted from `terraphim-ai` in commit `aa7ba99e8`. To apply the fix: |
| 260 | + |
| 261 | +1. Restore from git history: `git show aa7ba99e8^:crates/terraphim-session-analyzer/` |
| 262 | +2. Or work in the publishing repo (if separate) |
| 263 | +3. Apply fix, bump version, publish to registry |
| 264 | +4. Update consumer version in terraphim-clients |
| 265 | + |
| 266 | +## Open Items |
| 267 | + |
| 268 | +| Item | Status | Owner | |
| 269 | +|------|--------|-------| |
| 270 | +| Confirm crate publishing repo (terraphim-ai history vs separate repo) | Pending | Investigation needed | |
| 271 | +| Investigate 427 `assistant` failures (may be separate issue) | Deferred | Future issue if pre-filter doesn't resolve them | |
0 commit comments