Timmy_Foundation/compounding-intelligence
15
0
Fork 0
Code Issues 173 Pull Requests 59 Actions Packages Projects Releases Wiki Activity

Add session_reader.py — JSONL transcript parser (#6) #21

Closed
Rockachopa wants to merge 1 commits from feat/session-reader into main
pull from: feat/session-reader
merge into: Timmy_Foundation:main
Conversation 1 Commits 1 Files Changed 1 +298
Rockachopa commented 2026-04-14 17:33:34 +00:00
Owner
Copy Link

Closes #6

Implements session_reader.py, a JSONL transcript parser for Hermes session files.

Features:

  • Parses JSONL format (one JSON object per line)
  • Separates user messages, assistant messages, and tool outputs
  • Extracts: session ID, model used, start/end time, total tokens
  • Classifies session by repo (scans messages for repo names)
  • Classifies outcome: success / partial / failure / unknown
  • Outputs structured session summary as JSON

Meets all acceptance criteria:

  • Reads any valid Hermes session JSONL
  • Correctly identifies repo from message content
  • Handles missing/malformed entries gracefully
  • Runs in <1 second per session file
Closes #6 Implements session_reader.py, a JSONL transcript parser for Hermes session files. Features: - Parses JSONL format (one JSON object per line) - Separates user messages, assistant messages, and tool outputs - Extracts: session ID, model used, start/end time, total tokens - Classifies session by repo (scans messages for repo names) - Classifies outcome: success / partial / failure / unknown - Outputs structured session summary as JSON Meets all acceptance criteria: - Reads any valid Hermes session JSONL - Correctly identifies repo from message content - Handles missing/malformed entries gracefully - Runs in <1 second per session file
Rockachopa added 1 commit 2026-04-14 17:33:35 +00:00
Add session_reader.py - JSONL transcript parser (#6) a20e9721b2
Timmy requested changes 2026-04-14 18:12:49 +00:00
Timmy left a comment
Owner
Copy Link

Review: PR #21 — Add session_reader.py — JSONL transcript parser (#6)

Recommendation: REQUEST_CHANGES — this is incompatible with the harvester and is a duplicate effort.

Duplicate / conflict analysis

PR #26 (and #20) already include a session_reader.py (142 lines) with the API that harvester.py depends on: read_session(), extract_conversation(), truncate_for_context(), messages_to_text(). This PR's session_reader.py (298 lines) has a completely different API surface built around a SessionSummary dataclass and parse_jsonl_session(). These two versions are incompatible. If this PR were merged, it would break the harvester.

Code feedback (in case parts are salvaged)

The code itself is well-written with some nice features:

Strengths:

  • SessionSummary dataclass is a clean abstraction
  • Repo classification via regex patterns is clever
  • Outcome classification logic is reasonable
  • Good error handling (FileNotFoundError, malformed JSON)
  • Duration estimation from timestamps
  • Token count estimation

Issues:

  1. Token estimation uses word count — total_tokens += len(content.split()) counts words, not tokens. This will undercount by roughly 30%. For an estimate this is acceptable, but the field name total_tokens_estimate could mislead. Consider renaming to word_count or adding a comment about the approximation.

  2. Repo detection stops at first match — The break after the first regex match means if an early generic pattern matches (e.g., a GitHub URL in an error message), the actual target repo might be missed. Consider scanning all messages and picking the most frequently mentioned repo.

  3. Outcome classification is simplistic — Checking the last message for success keywords like 'done' or 'completed' will produce false positives on messages like 'I have not completed this yet'. Consider checking for these words in more specific contexts.

  4. No tests included — The PR description claims it meets all acceptance criteria but provides no test file.

  5. process_session_directory only matches session_*.jsonl — This glob pattern may miss session files with different naming conventions.

Recommendation

Close this PR. The session_reader.py in PR #26 is the version that integrates with the harvester pipeline. However, the SessionSummary dataclass and metadata extraction logic from this PR is genuinely useful — consider extracting it into a separate session_metadata.py module and opening a new PR for that, designed to complement (not replace) the existing session_reader.py.

## Review: PR #21 — Add session_reader.py — JSONL transcript parser (#6) **Recommendation: REQUEST_CHANGES — this is incompatible with the harvester and is a duplicate effort.** ### Duplicate / conflict analysis PR #26 (and #20) already include a session_reader.py (142 lines) with the API that harvester.py depends on: `read_session()`, `extract_conversation()`, `truncate_for_context()`, `messages_to_text()`. This PR's session_reader.py (298 lines) has a completely different API surface built around a `SessionSummary` dataclass and `parse_jsonl_session()`. **These two versions are incompatible.** If this PR were merged, it would break the harvester. ### Code feedback (in case parts are salvaged) The code itself is well-written with some nice features: **Strengths:** - `SessionSummary` dataclass is a clean abstraction - Repo classification via regex patterns is clever - Outcome classification logic is reasonable - Good error handling (FileNotFoundError, malformed JSON) - Duration estimation from timestamps - Token count estimation **Issues:** 1. **Token estimation uses word count** — `total_tokens += len(content.split())` counts words, not tokens. This will undercount by roughly 30%. For an estimate this is acceptable, but the field name `total_tokens_estimate` could mislead. Consider renaming to `word_count` or adding a comment about the approximation. 2. **Repo detection stops at first match** — The `break` after the first regex match means if an early generic pattern matches (e.g., a GitHub URL in an error message), the actual target repo might be missed. Consider scanning all messages and picking the most frequently mentioned repo. 3. **Outcome classification is simplistic** — Checking the last message for success keywords like 'done' or 'completed' will produce false positives on messages like 'I have not completed this yet'. Consider checking for these words in more specific contexts. 4. **No tests included** — The PR description claims it meets all acceptance criteria but provides no test file. 5. **`process_session_directory` only matches `session_*.jsonl`** — This glob pattern may miss session files with different naming conventions. ### Recommendation Close this PR. The session_reader.py in PR #26 is the version that integrates with the harvester pipeline. However, the `SessionSummary` dataclass and metadata extraction logic from this PR is genuinely useful — consider extracting it into a separate `session_metadata.py` module and opening a new PR for that, designed to complement (not replace) the existing session_reader.py.
Rockachopa closed this pull request 2026-04-14 19:01:55 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Timmy
Labels
Clear labels
acceptance-criteria
batch-pipeline
Token masterplan batch pipeline
 bootstrapper
Pre-session context injection
 epic
Epic-level issue
 harvester
Session knowledge extraction
 measurer
Compounding metrics
 milestone:1
Milestone 1: Foundation
 milestone:2
Milestone 2: Integration
 milestone:3
Milestone 3: Measurement
 milestone:4
Milestone 4: Retroactive
 pipeline
Pipeline/integration work
 pipeline
priority:high
priority:medium
retroactive
Processing existing sessions
 throughput-10x
throughput-10x label
 token-masterplan
token-masterplan label
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
Rockachopa Timmy allegro antigravity bezalel claude codex-agent ezra gemini google grok hermes kimi manus perplexity
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Timmy_Foundation/compounding-intelligence#21
Delete Branch "feat/session-reader"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?