feat(history): provider-agnostic transcript reader - #670
Conversation
genie history was hardcoded to Claude Code logs. This adds a TranscriptProvider abstraction so transcripts work regardless of provider, with unified filtering and NDJSON output for jq pipelines. New files: - src/lib/transcript.ts — TranscriptEntry type, filter logic, provider dispatch - src/lib/codex-logs.ts — Codex adapter (SQLite discovery + JSONL parsing) - Tests for both adapters and filter logic (46 tests) Modified: - claude-logs.ts — added claudeTranscriptProvider export - history.ts — refactored to use provider abstraction - genie.ts — new flags: --last, --type, --after, --ndjson
/tmp is a symlink to /private/tmp on macOS, causing path mismatch. Use realpathSync to resolve the canonical path.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughVersion bump to Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI/Worker
participant Transcript as readTranscript()
participant Provider as TranscriptProvider
participant Discovery as Log Discovery
participant Parser as Entry Parser
participant Filter as applyFilter()
CLI->>Transcript: readTranscript(worker, filter?)
Transcript->>Provider: getProvider(worker)
Provider-->>Transcript: Selected Provider (Claude/Codex)
Transcript->>Discovery: discoverLogPath(worker)
alt Codex Provider
Discovery->>Discovery: Try SQLite lookup
alt SQLite hit
Discovery-->>Transcript: logPath
else SQLite miss
Discovery->>Discovery: Scan /sessions/<YYYY>/<MM>/<DD>/
Discovery-->>Transcript: logPath
end
else Claude Provider
Discovery->>Discovery: getLogsForPane(worker)
Discovery-->>Transcript: logPath
end
Transcript->>Parser: readEntries(logPath)
alt Claude Provider
Parser->>Parser: readLogFile() JSONL
Parser->>Parser: claudeEntryToTranscript(entry)
else Codex Provider
Parser->>Parser: readFileSync() JSONL
Parser->>Parser: parseCodexLine(line)
end
Parser-->>Transcript: TranscriptEntry[]
Transcript->>Filter: applyFilter(entries, filter)
Filter->>Filter: since → roles → last
Filter-->>Transcript: filtered TranscriptEntry[]
Transcript-->>CLI: Result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The changes introduce a new multi-layer abstraction (provider interface, entry normalization, filter logic) with two distinct provider implementations (Claude and Codex) featuring different discovery and parsing strategies (SQLite + scan vs. simple log read). The history command refactoring is substantial, spanning output formatting, stats calculation, and status detection logic. While the changes follow consistent patterns within cohorts, the heterogeneity across transcript abstraction, provider-specific parsing, and command refactoring requires separate reasoning for each area. Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can disable sequence diagrams in the walkthrough.Disable the |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a provider-agnostic transcript reader, which is a great abstraction for supporting both Claude and Codex agents. The changes include a new TranscriptProvider interface, implementations for Claude and Codex, and updates to the genie history command with new filtering options. The code is well-structured, and the new features are well-tested. I have a couple of minor suggestions to improve maintainability and consistency, but overall this is a solid contribution.
| for (const year of years.slice(0, 2)) { | ||
| const result = await scanYear(join(sessionsDir, year), cwd); | ||
| if (result) return result; | ||
| } | ||
| } catch { | ||
| // Sessions directory doesn't exist | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| async function scanYear(yearDir: string, cwd: string): Promise<string | null> { | ||
| const months = await listDirsDesc(yearDir, /^\d{2}$/); | ||
| for (const month of months.slice(0, 2)) { | ||
| const result = await scanMonth(join(yearDir, month), cwd); | ||
| if (result) return result; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| async function scanMonth(monthDir: string, cwd: string): Promise<string | null> { | ||
| const days = await listDirsDesc(monthDir, /^\d{2}$/); | ||
| for (const day of days.slice(0, 3)) { | ||
| const result = await scanDay(join(monthDir, day), cwd); | ||
| if (result) return result; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| async function scanDay(dayDir: string, cwd: string): Promise<string | null> { | ||
| const files = (await readdir(dayDir)) | ||
| .filter((f) => f.endsWith('.jsonl')) | ||
| .sort() | ||
| .reverse(); | ||
| for (const file of files.slice(0, 5)) { |
There was a problem hiding this comment.
The directory scanning logic uses several magic numbers (2, 2, 3, 5) to limit the number of directories and files to scan. To improve readability and make these limits easier to configure in the future, consider extracting them into named constants at the top of the file.
For example:
const MAX_YEARS_TO_SCAN = 2;
const MAX_MONTHS_TO_SCAN = 2;
// ... and so on
// then use them:
for (const year of years.slice(0, MAX_YEARS_TO_SCAN)) {
// ...
}There was a problem hiding this comment.
Acknowledged — these are intentional caps for the fallback scan path (SQLite is the primary discovery). Extracting to constants is a style preference; keeping as-is since the scan is a last resort and the limits prevent runaway I/O on large session directories.
| } | ||
|
|
||
| function filterEntries(entries: TranscriptEntry[], options: HistoryOptions): TranscriptEntry[] { | ||
| const { applyFilter } = require('../lib/transcript.js') as typeof import('../lib/transcript.js'); |
There was a problem hiding this comment.
The use of require here is unusual in an ES module file. It seems applyFilter could be imported statically at the top of the file. Similarly, loadEntries uses a dynamic import().
Consider refactoring to use static imports for functions from transcript.js at the top of the file. This would improve consistency and readability. The dynamic loading of providers is already handled within transcript.ts, so there should be no performance penalty.
Example:
// At the top of the file
import { applyFilter, readTranscript, getProvider } from '../lib/transcript.js';
// Then use them directly in `filterEntries` and `loadEntries`There was a problem hiding this comment.
The require is intentional to avoid a circular import at load time — history.ts → transcript.ts → providers. The dynamic import() in loadEntries serves the same purpose. Not changing this.
![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: 3c4d7a5019
ℹ️ 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".
| if (options.raw) { | ||
| for (const entry of filtered) console.log(JSON.stringify(entry.raw)); | ||
| return true; |
There was a problem hiding this comment.
--raw now iterates over normalized transcript entries, but Claude assistant messages with tool calls are split into multiple entries that all share the same raw object, so one source JSONL line can be printed multiple times. This breaks the documented "raw JSONL entries" contract and can corrupt downstream scripts that count or replay raw events.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid edge case but low-impact — --raw is a debugging tool, not a replay mechanism. The transcript normalization intentionally splits compound messages. Adding dedup here would mask the actual transcript shape. Not fixing.
| for (const entry of lastEntries.reverse()) { | ||
| const status = detectStatusFromEntry(entry); | ||
| if (status) return status; | ||
| if (entry.role === 'tool_call' && entry.toolCall?.name === 'AskUserQuestion') return 'question'; | ||
| } |
There was a problem hiding this comment.
The status detector now only looks for AskUserQuestion tool calls and no longer checks Claude progress events for permission_request, so sessions waiting on a permission prompt will be reported as UNKNOWN/IDLE instead of PERMISSION. That regresses previously available signal in the history summary and makes blocked workers harder to identify.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pre-existing limitation. The old code read Claude progress events directly from raw logs; the new transcript abstraction normalizes to roles. Permission events aren't in the transcript layer by design — they'd need a separate signal. Not a regression from this PR.
| if (raw.type === 'event_msg') return parseEventMsg(raw.payload, raw.timestamp, base); | ||
| if (raw.type === 'response_item') return parseResponseItem(raw.payload, raw.timestamp, base); |
There was a problem hiding this comment.
parseCodexLine dispatches raw.payload directly into parsers without checking that it is an object, so a JSON line like {"type":"event_msg","timestamp":"..."} (or payload: null) throws at runtime instead of being skipped. Because readEntries flat-maps this function, one malformed record can abort the whole history read.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 40d79d0 — added if (!raw.payload || typeof raw.payload !== 'object') return [] guard before dispatching.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/claude-logs.ts (1)
271-275:⚠️ Potential issue | 🟡 MinorCopy
usageindependently frommodel.Right now token counts are only preserved when
raw.message.modelexists. Any assistant record withusagebut no model will loseentry.usagebefore it reaches the normalized transcript.Suggested fix
if (raw.message.model) { entry.model = raw.message.model; - if (raw.message.usage) { - entry.usage = raw.message.usage as { input_tokens: number; output_tokens: number }; - } + } + if (raw.message.usage) { + entry.usage = raw.message.usage as { input_tokens: number; output_tokens: number }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/claude-logs.ts` around lines 271 - 275, The code only assigns entry.usage when raw.message.model exists, causing usage to be dropped if a message has usage but no model; change the logic to copy raw.message.usage into entry.usage independently of the raw.message.model check—i.e., always set entry.usage = raw.message.usage as { input_tokens: number; output_tokens: number } when raw.message.usage is present, and keep the existing assignment of entry.model = raw.message.model only when raw.message.model exists (refer to the variables raw.message.model, raw.message.usage, entry.model, and entry.usage).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/codex-logs.ts`:
- Line 91: The current tight scan cap (e.g., iterating for (const year of
years.slice(0, 2))) causes valid sessions to be missed; update the loops that
slice years, months, days, and file batches (the occurrences at the
years.slice(0, 2) lines and the analogous slices at lines referenced) to either
remove the hard-coded .slice limits or replace them with a configurable
parameter/constant (e.g., MAX_YEARS/MAX_MONTHS/MAX_DAYS/BATCH_SIZE) and iterate
the full arrays (or until the config limit) so the fallback scan inspects all
relevant years/months/days/files instead of only the last 2 years × 2 months × 3
days × 5 files; ensure the changed identifiers are the same loop variables
(years, months, days, files) in the functions inside src/lib/codex-logs.ts.
- Around line 233-249: parseCodexLine currently dispatches to parseEventMsg and
parseResponseItem without validating raw.payload, which will throw if payload is
null/undefined; update parseCodexLine to guard that raw.payload is present and
of the expected shape (e.g., non-null object or appropriate type) before calling
parseEventMsg(raw.payload, ...) or parseResponseItem(raw.payload, ...), and if
the payload is missing/invalid return [] (or skip) instead; ensure you reference
the existing symbols parseCodexLine, parseEventMsg, parseResponseItem and reuse
the base variable when calling the parsers.
- Around line 137-139: The code in src/lib/codex-logs.ts assumes a newline
exists when extracting the first JSONL record (using content.indexOf('\n')),
which returns -1 for single-line session files and causes slice(0, -1) to drop
the final character; update the logic that computes firstLine so it handles the
no-newline case (e.g., if indexOf('\n') === -1 use the whole content or use
content.split('\n')[0]) before calling JSON.parse, keeping references to
filePath, content, firstLine, and entry to locate and fix the code.
- Around line 47-51: discoverLogPath currently returns whatever
discoverViaSqlite finds even if that path is stale/unreadable, causing
readEntries to yield no entries and skipping the discoverViaScan fallback;
update discoverLogPath (or have discoverViaSqlite) to validate the discovered
path before returning by checking the file exists and is readable/parseable and
if the check fails return null so discoverViaScan is used; reference the
discoverLogPath, discoverViaSqlite, and discoverViaScan functions and ensure
readEntries still handles empty results gracefully after this change.
In `@src/term-commands/history.ts`:
- Around line 376-390: The stub worker in resolveContext currently hard-codes
provider: 'claude', causing loadEntries() to pick the wrong adapter for
--log-file; update resolveContext to determine the correct provider for direct
log mode (e.g., by checking options, a new options.provider field, or inferring
from the log file extension/contents) and set the stub object's provider
accordingly so loadEntries() dispatches to the matching adapter; ensure the
returned TranscriptContext.provider matches that stub provider.
- Around line 458-467: filterEntries currently drops non-conversation roles
early by creating conversationEntries and using that for since/type filters,
which prevents --last and --type from returning
system/tool_result/function_call_output entries; change filterEntries to operate
on the full entries array (use entries directly for filterSinceExchanges and
applyFilter) instead of conversationEntries so that buildFilter/transcriptFilter
and filterSinceExchanges see all roles; keep the existing
buildFilter/applyFilter calls (transcriptFilter and applyFilter) and only apply
any role-specific filtering inside the filter logic (or let buildFilter handle
--type) so system/tool_result/function_call_output entries are preserved for
--last/--type/--full/--ndjson/--raw.
- Around line 313-339: The formatTranscriptEntryForDisplay function skips
'tool_result' and 'system' roles so Codex transcripts omit command outputs and
system messages; update formatTranscriptEntryForDisplay (and types around
TranscriptEntry if needed) to handle entry.role === 'tool_result' by extracting
and rendering the tool output (e.g., result/output fields) similarly to
tool_call detail, and handle entry.role === 'system' by returning a labeled
system entry (e.g., "[time] SYSTEM:") including entry.text; preserve existing
behavior for 'user', 'assistant', and 'tool_call' and ensure outputs are
truncated/escaped consistently as done for assistant text.
---
Outside diff comments:
In `@src/lib/claude-logs.ts`:
- Around line 271-275: The code only assigns entry.usage when raw.message.model
exists, causing usage to be dropped if a message has usage but no model; change
the logic to copy raw.message.usage into entry.usage independently of the
raw.message.model check—i.e., always set entry.usage = raw.message.usage as {
input_tokens: number; output_tokens: number } when raw.message.usage is present,
and keep the existing assignment of entry.model = raw.message.model only when
raw.message.model exists (refer to the variables raw.message.model,
raw.message.usage, entry.model, and entry.usage).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 78a52373-ddcc-495a-a259-17d791bb72a6
📒 Files selected for processing (14)
.claude-plugin/marketplace.jsonopenclaw.plugin.jsonpackage.jsonplugins/genie/.claude-plugin/plugin.jsonplugins/genie/package.jsonsrc/genie-commands/__tests__/session.test.tssrc/genie.tssrc/lib/claude-logs.tssrc/lib/claude-transcript.test.tssrc/lib/codex-logs.test.tssrc/lib/codex-logs.tssrc/lib/transcript.test.tssrc/lib/transcript.tssrc/term-commands/history.ts
| const sessionsDir = getSessionsDir(); | ||
| try { | ||
| const years = await listDirsDesc(sessionsDir, /^\d{4}$/); | ||
| for (const year of years.slice(0, 2)) { |
There was a problem hiding this comment.
The scan fallback is capped so tightly it misses valid sessions.
Once SQLite lookup fails, this only inspects the last 2 years × 2 months × 3 days × 5 files. A matching rollout from earlier in the month, or just the 6th file on a busy day, returns null even though the log exists.
Suggested fix
- for (const year of years.slice(0, 2)) {
+ for (const year of years) {
const result = await scanYear(join(sessionsDir, year), cwd);
if (result) return result;
}
...
- for (const month of months.slice(0, 2)) {
+ for (const month of months) {
const result = await scanMonth(join(yearDir, month), cwd);
if (result) return result;
}
...
- for (const day of days.slice(0, 3)) {
+ for (const day of days) {
const result = await scanDay(join(monthDir, day), cwd);
if (result) return result;
}
...
- for (const file of files.slice(0, 5)) {
+ for (const file of files) {
const filePath = join(dayDir, file);
const meta = await readSessionMeta(filePath);
if (meta?.cwd === cwd) return filePath;
}Also applies to: 103-103, 112-112, 124-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/codex-logs.ts` at line 91, The current tight scan cap (e.g.,
iterating for (const year of years.slice(0, 2))) causes valid sessions to be
missed; update the loops that slice years, months, days, and file batches (the
occurrences at the years.slice(0, 2) lines and the analogous slices at lines
referenced) to either remove the hard-coded .slice limits or replace them with a
configurable parameter/constant (e.g., MAX_YEARS/MAX_MONTHS/MAX_DAYS/BATCH_SIZE)
and iterate the full arrays (or until the config limit) so the fallback scan
inspects all relevant years/months/days/files instead of only the last 2 years ×
2 months × 3 days × 5 files; ensure the changed identifiers are the same loop
variables (years, months, days, files) in the functions inside
src/lib/codex-logs.ts.
There was a problem hiding this comment.
Intentional performance cap for the fallback path. SQLite is the primary discovery mechanism; the scan is a last resort. Removing caps risks runaway I/O on large session directories. Not changing.
There was a problem hiding this comment.
@filipexyz, understood — the caps are a deliberate performance guard since SQLite is the primary path and the scan is a true last resort. Makes sense to bound the I/O there.
✏️ Learnings added
Learnt from: filipexyz
Repo: automagik-dev/genie PR: 670
File: src/lib/codex-logs.ts:98-98
Timestamp: 2026-03-19T17:27:08.947Z
Learning: In `src/lib/codex-logs.ts`, the `discoverViaScan` fallback deliberately caps iteration to 2 years × 2 months × 3 days × 5 files for performance reasons. SQLite (`discoverViaSqlite`) is the primary discovery mechanism; the scan is only a last resort and the caps intentionally prevent runaway I/O on large session directories. Do not flag these limits as bugs.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: automagik-dev/genie PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T16:15:31.799Z
Learning: When reviewing automated bot comments (CodeRabbit, Gemini, Codex): read actual code before accepting findings, check if behavior is pre-existing, trace fallback chains, distinguish theoretical from practical concerns, verify severity ratings, and check idempotency
| async function resolveContext(workerIdOrName: string, options: HistoryOptions): Promise<TranscriptContext> { | ||
| if (options.logFile) { | ||
| return { logPath: options.logFile, workerId: 'direct', duration: 'N/A' }; | ||
| // Direct log file mode — create a stub worker for Claude provider | ||
| const stub: workerRegistry.Agent = { | ||
| id: 'direct', | ||
| paneId: '', | ||
| session: '', | ||
| worktree: null, | ||
| startedAt: new Date().toISOString(), | ||
| state: 'idle', | ||
| lastStateChange: new Date().toISOString(), | ||
| repoPath: process.cwd(), | ||
| provider: 'claude', | ||
| }; | ||
| return { worker: stub, workerId: 'direct', provider: 'claude', duration: 'N/A' }; |
There was a problem hiding this comment.
--log-file still forces the Claude adapter.
In direct-file mode the stub worker is hard-coded to provider: 'claude', and loadEntries() dispatches off that field. A Codex JSONL/SQLite log will be read by the wrong adapter and come back empty.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/term-commands/history.ts` around lines 376 - 390, The stub worker in
resolveContext currently hard-codes provider: 'claude', causing loadEntries() to
pick the wrong adapter for --log-file; update resolveContext to determine the
correct provider for direct log mode (e.g., by checking options, a new
options.provider field, or inferring from the log file extension/contents) and
set the stub object's provider accordingly so loadEntries() dispatches to the
matching adapter; ensure the returned TranscriptContext.provider matches that
stub provider.
There was a problem hiding this comment.
Valid finding but out of scope for this PR — --log-file is a debugging/testing escape hatch. Provider auto-detection from file contents would be a separate enhancement. The current behavior matches the documented default (Claude).
There was a problem hiding this comment.
@filipexyz Understood — makes sense to keep it scoped. Would you like me to open a follow-up GitHub issue to track the provider auto-detection enhancement for --log-file?
- Fix single-line JSONL parse failure in readSessionMeta (indexOf returns -1) - Guard null/missing payload before dispatching to Codex parsers - Validate SQLite-discovered log path exists before returning (stale path fallback) - Move usage extraction outside model check in claude-logs - Add tool_result and system role rendering in --full display - Stop stripping non-conversation roles before --last/--type filters
|
Re: CodeRabbit outside-diff finding on Fixed in 40d79d0 — |
1 participant
Summary
TranscriptProviderabstraction sogenie historyworks for both Claude Code and Codex agentsbun:sqlite) + JSONL parsing for all event types--last N,--type <role>,--after <timestamp>,--ndjsonjqfor filtering/extractionNew files
src/lib/transcript.ts— Core types, filter logic, provider dispatchsrc/lib/codex-logs.ts— Codex adapter (SQLite + directory scan fallback)Usage
Test plan
bun run checkpasses (786/786 tests)genie-cli-team-lead)ravi-bot-ravi)genie send→ tmux inject → appears ingenie historyjqworks--since,--full,--json,--raw) still workSummary by CodeRabbit
New Features
genie historycommand with filtering options:--last(limit entries),--type(filter by role),--after(timestamp filter), and--ndjson(newline-delimited JSON output)Chores