feat: wire ObservationStore and Claude ingestion into Electron - #120
feat: wire ObservationStore and Claude ingestion into Electron#120Coldaine wants to merge 3 commits into
Conversation
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
| const stat = fs.statSync(this.filePath) | ||
| this.fileSize = 0 | ||
| this.tail = '' | ||
| this.readFrom(0) | ||
| this.fileSize = stat.size | ||
|
|
There was a problem hiding this comment.
Suggestion: Startup has a stale-size race: readFrom(0) performs a separate stat and may consume bytes appended after the earlier statSync, but this assignment then restores the older size. The next poll rereads those already-emitted bytes and produces duplicate events. Preserve the size returned by readFrom or obtain and use a consistent file snapshot. [race condition]
Severity Level: Major ⚠️
- ⚠️ Live Claude transcript ingestion can emit duplicate lines.
- ⚠️ ObservationStore may retain duplicate agent observations.
- ⚠️ Electron renderer can receive duplicate event notifications.Steps of Reproduction ✅
1. Start Electron and send the renderer ready message;
`electron/ingestion-host.ts:142-174` calls `startPlanned()` after selecting the latest
Claude transcript.
2. For a live transcript, `electron/ingestion-host.ts:207-216` calls
`IngestionAdapter.startDriver('claude-code', ...)` with `replay: false` and transcript
mode.
3. `host/ingestion/src/drivers/claude-code.ts:101-112` creates `JsonlTailSource` and
invokes `source.start()`, which records the size at `jsonl-tail.ts:49`, then calls
`readFrom(0)` at line 52.
4. If Claude appends a complete JSONL record after the outer `statSync()` but before
`readNewFileLines()` performs its internal `statSync()` at
`extension/src/fs-utils.ts:31-44`, `readFrom(0)` consumes and emits that record, then
`jsonl-tail.ts:54` restores the older size.
5. On the next filesystem change or 500 ms poll (`jsonl-tail.ts:56-63`), `readNew()`
starts from the stale offset at lines 86-88, rereads the same record, and sends it through
`ClaudeCodeDriver.ingestLine()` at `claude-code.ts:62-67`; the adapter then stores and
forwards the duplicate through `ingestion-adapter.ts:105-115`.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/ingestion/src/jsonl-tail.ts
**Line:** 49:54
**Comment:**
*Race Condition: Startup has a stale-size race: `readFrom(0)` performs a separate stat and may consume bytes appended after the earlier `statSync`, but this assignment then restores the older size. The next poll rereads those already-emitted bytes and produces duplicate events. Preserve the size returned by `readFrom` or obtain and use a consistent file snapshot.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| private readonly subscribers = new Set<ObservationSubscriber>() | ||
|
|
||
| constructor(options: ObservationStoreOptions = {}) { | ||
| this.capacity = Math.max(1, options.capacity ?? DEFAULT_CAPACITY) |
There was a problem hiding this comment.
Suggestion: The configured capacity is not validated for NaN. With capacity: NaN, the eviction condition is always false, so the supposedly bounded ring buffer grows without limit in the long-lived Electron process. Normalize the value to a finite positive integer before storing it. [memory leak]
Severity Level: Critical 🚨
- ❌ Configured stores can retain unlimited observations.
- ❌ Long-lived Electron ingestion can exhaust memory.
- ⚠️ Default Electron construction uses capacity 5,000.Steps of Reproduction ✅
1. Construct `IngestionAdapter` with `storeOptions: { capacity: NaN }`;
`host/ingestion/src/ingestion-adapter.ts:44-46` forwards these options to `new
ObservationStore(options.storeOptions)`.
2. `ObservationStore` evaluates `Math.max(1, NaN)` at
`host/ingestion/src/observation-store.ts:56-57`, producing `NaN` for `this.capacity`.
3. Push events through `IngestionAdapter.push()` at
`host/ingestion/src/ingestion-adapter.ts:76-86`; each call appends to the store.
4. The eviction loop at `host/ingestion/src/observation-store.ts:82-85` checks
`buffer.length > NaN`, which is always false, so the buffer grows without bound instead of
enforcing its configured limit.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/ingestion/src/observation-store.ts
**Line:** 57:57
**Comment:**
*Memory Leak: The configured capacity is not validated for `NaN`. With `capacity: NaN`, the eviction condition is always false, so the supposedly bounded ring buffer grows without limit in the long-lived Electron process. Normalize the value to a finite positive integer before storing it.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if (hay.includes(needle)) { | ||
| hits.push(obs) | ||
| if (hits.length >= limit) break | ||
| } |
There was a problem hiding this comment.
Suggestion: limit: 0 and negative limits still return the first matching observation because the limit is checked only after pushing a hit. Callers using the documented query API therefore receive rows when they requested none, or receive one row for an invalid negative limit. Return early for non-positive limits or check the limit before adding a result. [logic error]
Severity Level: Major ⚠️
- ⚠️ Curator transcript searches return unexpected rows.
- ⚠️ Zero-result pagination requests are violated.
- ⚠️ Negative limits produce misleading search results.Steps of Reproduction ✅
1. Append a matching message through `ObservationStore.append()` at
`host/ingestion/src/observation-store.ts:65-89`, or through `IngestionAdapter.push()` at
`host/ingestion/src/ingestion-adapter.ts:76-86`.
2. Call the public query method `searchTranscript('migration', { limit: 0 })` defined at
`host/ingestion/src/observation-store.ts:132-135`.
3. The method accepts the zero limit at `host/ingestion/src/observation-store.ts:139-140`,
then reaches the matching event.
4. At `host/ingestion/src/observation-store.ts:145-148`, it pushes the match before
checking `hits.length >= limit`, so the result contains one observation instead of zero.
Negative limits likewise return one match. Current Electron IPC sanitizes `queryRecent`
counts at `electron/main.ts:76-79`, but does not change this search API behavior.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/ingestion/src/observation-store.ts
**Line:** 145:148
**Comment:**
*Logic Error: `limit: 0` and negative limits still return the first matching observation because the limit is checked only after pushing a hit. Callers using the documented query API therefore receive rows when they requested none, or receive one row for an invalid negative limit. Return early for non-positive limits or check the limit before adding a result.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| getAll(): StoredObservation[] { | ||
| return this.buffer.slice() | ||
| } |
There was a problem hiding this comment.
Suggestion: getAll() returns the actual stored observation objects rather than immutable copies. The adapter exposes this through its read-only query handle, and callers can mutate event, payload, or nested payload values, changing historical observations still retained in the ring buffer and the results of later queries. Return defensive copies or make the stored observation graph immutable. [stale reference]
Severity Level: Major ⚠️
- ❌ Curator evidence can be changed after ingestion.
- ⚠️ Later searches can observe corrupted historical payloads.
- ⚠️ Electron fixture session stamping mutates retained observations.Steps of Reproduction ✅
1. Obtain the documented read-only query handle through `IngestionAdapter.query` at
`host/ingestion/src/ingestion-adapter.ts:52-55`; it returns `ObservationStore.asQuery()`
at `host/ingestion/src/observation-store.ts:157-160`.
2. Append an observation whose payload contains nested data using
`ObservationStore.append()` at `host/ingestion/src/observation-store.ts:65-89`.
3. Call `query.getAll()`. The implementation at
`host/ingestion/src/observation-store.ts:153-155` copies only the outer array, leaving
each `StoredObservation`, `event`, and nested payload value shared with the store.
4. Mutate the returned observation or nested payload, then call
`query.searchTranscript()`, `query.byType()`, or `query.getAll()` again; those methods
read the same retained objects and observe the mutation. The Electron host already
demonstrates direct mutation through this query surface at
`electron/ingestion-host.ts:228-233` while stamping fixture session IDs, so defensive
copying requires refactoring that mutation rather than copying only the array.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/ingestion/src/observation-store.ts
**Line:** 153:155
**Comment:**
*Stale Reference: `getAll()` returns the actual stored observation objects rather than immutable copies. The adapter exposes this through its read-only query handle, and callers can mutate `event`, `payload`, or nested payload values, changing historical observations still retained in the ring buffer and the results of later queries. Return defensive copies or make the stored observation graph immutable.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| async onRendererReady(): Promise<IngestionHostStatus> { | ||
| this.adapter.stopAll() | ||
| this.adapter.store.clear() |
There was a problem hiding this comment.
Suggestion: Each ready message starts an independent asynchronous initialization without serializing or cancelling earlier calls. If the renderer sends ready again while the first startPlanned() is awaiting driver startup, the second call clears and restarts the store, while the first call can later replay or forward events using the shared status and forwardToRenderer fields. This produces duplicate, interleaved, or incorrectly session-stamped events after renderer reloads or reconnects. Guard initialization with a generation token or serialize readiness handling so only the latest startup can mutate state or send events. [race condition]
Severity Level: Critical 🚨
- ❌ Renderer reloads can duplicate fixture events.
- ❌ Concurrent starts can clear active observation history.
- ⚠️ Session stamping uses shared mutable status state.
- ⚠️ Duplicate batches inflate visualizer simulation state.Steps of Reproduction ✅
1. The Electron renderer sends `{ type: 'ready' }` from `web/lib/electron-bridge.ts:46-47`
after configuring the bridge; a renderer reload or repeated bridge initialization can send
another ready message before the first initialization promise completes.
2. Each ready message enters `electron/main.ts:44-56`, invokes
`IngestionHost.onRendererReady()`, and starts the asynchronous operation without
serialization.
3. The first invocation clears the store and starts a driver at
`electron/ingestion-host.ts:141-172`; the second invocation can then execute `stopAll()`,
clear the same store, replace `status`, and start another replay.
4. For the fixture path, `host/ingestion/src/drivers/claude-code.ts:100-120` performs
synchronous `replayOnce()` work before returning, then `onRendererReady()` resumes at
`electron/ingestion-host.ts:228-237`. The earlier invocation can therefore read and send
the store contents created by the later invocation, while the later invocation sends them
again, producing duplicate `agent-event-batch` messages.
5. The renderer processes every event in each batch at `web/lib/vscode-bridge.ts:53-59`,
so duplicated or stale batches are applied to the selected simulation session.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electron/ingestion-host.ts
**Line:** 141:143
**Comment:**
*Race Condition: Each `ready` message starts an independent asynchronous initialization without serializing or cancelling earlier calls. If the renderer sends `ready` again while the first `startPlanned()` is awaiting driver startup, the second call clears and restarts the store, while the first call can later replay or forward events using the shared `status` and `forwardToRenderer` fields. This produces duplicate, interleaved, or incorrectly session-stamped events after renderer reloads or reconnects. Guard initialization with a generation token or serialize readiness handling so only the latest startup can mutate state or send events.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| void ingestion?.onRendererReady().catch((err) => { | ||
| console.error('[ingestion-host] failed to start:', err) | ||
| sendToRenderer({ | ||
| type: 'config', | ||
| config: { showMockData: true, mode: 'live', autoPlay: true }, | ||
| }) | ||
| sendToRenderer({ | ||
| type: 'connection-status', | ||
| status: 'disconnected', | ||
| source: 'electron-shell', | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Suggestion: The startup error handler only changes the renderer configuration and connection status. onRendererReady() has already sent session-started and assigned IngestionHost.status before startPlanned() can reject, so a failed driver start leaves the renderer with an active selected session while reporting a disconnected/mock connection. It also does not stop a partially created tail. Reset or end the announced session and dispose the failed ingestion attempt before sending the fallback state. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Failed ingestion leaves stale active session selected.
- ❌ Renderer state contradicts disconnected connection status.
- ⚠️ Mock fallback does not clear ingestion observations.
- ⚠️ Partial tail resources may remain until quit.Steps of Reproduction ✅
1. The renderer sends `ready` through `web/lib/electron-bridge.ts:46-47`, and
`electron/main.ts:44-56` invokes `onRendererReady()` asynchronously.
2. `electron/ingestion-host.ts:146-170` assigns `this.status` and sends `session-started`
with status `active` before calling `startPlanned()` at line 172.
3. A planned transcript can become unavailable or inaccessible between discovery at
`electron/ingestion-host.ts:181-189` and driver startup.
`host/ingestion/src/drivers/claude-code.ts:100-120` constructs a `JsonlTailSource` and
starts it; `host/ingestion/src/jsonl-tail.ts:25-35` can then throw during filesystem setup
or stat/read operations.
4. The rejected promise reaches the catch handler at `electron/main.ts:45-56`, which only
sends fallback config and disconnected status. No `session-ended`, `reset`, or equivalent
lifecycle message is sent, and `IngestionHost.status` remains populated.
5. The renderer's session handler at `web/hooks/use-vscode-bridge.ts:203-219` has already
added and selected the active session, while the fallback handler only changes
`useMockData` at `web/hooks/use-vscode-bridge.ts:160-166`; the UI can therefore retain the
failed active session alongside disconnected/mock state.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electron/main.ts
**Line:** 45:56
**Comment:**
*Incomplete Implementation: The startup error handler only changes the renderer configuration and connection status. `onRendererReady()` has already sent `session-started` and assigned `IngestionHost.status` before `startPlanned()` can reject, so a failed driver start leaves the renderer with an active selected session while reporting a disconnected/mock connection. It also does not stop a partially created tail. Reset or end the announced session and dispose the failed ingestion attempt before sending the fallback state.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| this.stop() | ||
| if (!options.filePath) return |
There was a problem hiding this comment.
Suggestion: Stopping the tail does not reset sessions, sessionStartTimes, or the TranscriptParser's per-session dedup state. Restarting the driver for the same transcript and session therefore treats replayed lines as already consumed and emits no events, which breaks renderer reconnects and repeated starts. Reset the parser/session state when starting a new ingestion run. [stale reference]
Severity Level: Critical 🚨
- ❌ Renderer reconnects can show an empty transcript.
- ❌ Repeated `ready` events suppress Claude history.
- ⚠️ Restarted sessions retain stale tool and message state.Steps of Reproduction ✅
1. Start Claude transcript ingestion through `IngestionHost.onRendererReady()` at
`electron/ingestion-host.ts:141-143`; the host stops any prior driver, clears the
ObservationStore, and then starts the same transcript through
`electron/ingestion-host.ts:210-215`.
2. `ClaudeCodeDriver.start()` calls only `this.stop()` at
`host/ingestion/src/drivers/claude-code.ts:100-102`; `stop()` at lines 124-127 disposes
the tail but does not clear `sessions`, `sessionStartTimes`, or parser state.
3. The first run creates the session in `ensureSession()` at
`host/ingestion/src/drivers/claude-code.ts:149-155`, and
`TranscriptParser.processTranscriptLine()` at `extension/src/transcript-parser.ts:98-129`
populates the session's `seenToolUseIds` and `seenMessageHashes`.
4. Trigger another renderer `ready` event, which follows the same restart path at
`electron/main.ts:43-46`. The new tail rereads the transcript from offset zero, but
`TranscriptParser.processTranscriptLine()` skips prior tool IDs at
`extension/src/transcript-parser.ts:113-118` and prior messages through the dedup set at
`extension/src/transcript-parser.ts:82-101`, so the cleared store receives no replayed
transcript events. The parser's private inline-subagent state also remains because
`clearSessionState()` is only called by `extension/src/session-watcher.ts:607-610`, never
by the Claude ingestion driver.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/ingestion/src/drivers/claude-code.ts
**Line:** 101:102
**Comment:**
*Stale Reference: Stopping the tail does not reset `sessions`, `sessionStartTimes`, or the `TranscriptParser`'s per-session dedup state. Restarting the driver for the same transcript and session therefore treats replayed lines as already consumed and emits no events, which breaks renderer reconnects and repeated starts. Reset the parser/session state when starting a new ingestion run.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if (mode === 'agent-events') { | ||
| const source = new JsonlTailSource(options.filePath, parseAgentEventLine) | ||
| this.tail = source | ||
| source.subscribe((event) => this.ingestEvent(event)) | ||
| if (options.replay) source.replayOnce() | ||
| else source.start() | ||
| return | ||
| } |
There was a problem hiding this comment.
Suggestion: The agent-events path ignores the sessionId supplied in HarnessDriverStartOptions and forwards events unchanged. Fixtures and other normalized event streams commonly omit sessionId, so replayed events are stored without the session identity even though the caller explicitly provided one. Stamp events with the configured session ID before emitting them, as the Codex driver does. [api mismatch]
Severity Level: Major ⚠️
- ⚠️ ObservationStore events lose configured session association.
- ⚠️ Session filtering cannot classify direct fixture replays.
- ⚠️ Electron currently relies on a separate post-replay stamping workaround.Steps of Reproduction ✅
1. Call the public `IngestionAdapter.startDriver()` path used by the Electron host at
`electron/ingestion-host.ts:210-215` and `electron/ingestion-host.ts:222-227`, supplying
`sessionId` together with `claudeMode: 'agent-events'`.
2. The Claude driver enters the agent-events branch at
`host/ingestion/src/drivers/claude-code.ts:107-114` and subscribes with
`source.subscribe((event) => this.ingestEvent(event))`; the configured `options.sessionId`
is not captured or applied.
3. Replay a normalized JSONL event that omits `sessionId`, as the repository fixture does
at `host/ingestion/fixtures/simulate-events.jsonl:1-8`; `ingestEvent()` at
`host/ingestion/src/drivers/claude-code.ts:129-131` forwards it unchanged.
4. `IngestionAdapter.push()` at `host/ingestion/src/ingestion-adapter.ts:105-114` stores
the event without a session identity. The current Electron fixture path applies a separate
workaround at `electron/ingestion-host.ts:99-105`, but direct adapter consumers and any
other agent-event caller still receive incorrectly unscoped observations.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/ingestion/src/drivers/claude-code.ts
**Line:** 107:114
**Comment:**
*Api Mismatch: The agent-events path ignores the `sessionId` supplied in `HarnessDriverStartOptions` and forwards events unchanged. Fixtures and other normalized event streams commonly omit `sessionId`, so replayed events are stored without the session identity even though the caller explicitly provided one. Stamp events with the configured session ID before emitting them, as the Codex driver does.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| "test": "node -e \"console.log('substrate PR: no host test packages yet'); process.exit(0)\"", | ||
| "test:live": "node -e \"console.log('test:live: skipped on substrate PR'); process.exit(0)\"" | ||
| "typecheck": "tsc -p electron/tsconfig.json", | ||
| "test:ingestion": "npm --prefix host/ingestion test", |
There was a problem hiding this comment.
CI installs only the root package, which has no workspaces and whose lockfile has no vitest entry. This script invokes the separate host/ingestion package where vitest is declared. The required CI run on this head already fails with vitest not found, so the test and subsequent build gates cannot run. Make ingestion a workspace/root dependency or install the nested package before invoking its test script.
|
|
||
| /** Read existing content and begin watching for appends. */ | ||
| start(): void { | ||
| if (!fs.existsSync(this.filePath)) { |
There was a problem hiding this comment.
The live Claude path is an observed-agent artifact. If it disappears after discovery (or cannot be opened), start() creates its parent directories and a new empty transcript file here. That violates the repository read-only observer contract and can alter the observed Claude state. Treat a missing file as disconnected/retryable instead; do not create or modify watched transcript files.
There was a problem hiding this comment.
Pull request overview
This PR adds a new host/ingestion runtime (ObservationStore + HarnessDriver registry) and wires it into the Electron main process so Claude Code transcript activity can stream live to the renderer (with a fixture fallback when no transcript is found).
Changes:
- Introduce
host/ingestionpackage: ObservationStore ring buffer + query surface, JSONL tailing, and Claude/Codex (stub) drivers. - Add
IngestionHostin Electron main to discover the latest~/.claude/projects/**.jsonl, start ingestion, and forward events over IPC (plusqueryRecentlookback). - Update build/test scripts: add esbuild bundling for Electron main/preload and enable ingestion test suite from repo root.
Reviewed changes
Copilot reviewed 25 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| web/lib/electron-bridge.ts | Extend Electron host API typing (optional queryRecent) |
| package.json | Build/test wiring; add esbuild bundling step |
| package-lock.json | Lockfile updates for esbuild |
| host/ingestion/vitest.config.ts | Vitest config + aliasing for tests |
| host/ingestion/tsconfig.json | Strict TS config for ingestion package |
| host/ingestion/tests/observation-store.test.ts | Tests for store capacity/query/search |
| host/ingestion/tests/ingestion-adapter.test.ts | Tests for adapter store + IPC forwarding + Claude replay |
| host/ingestion/tests/drivers.test.ts | Tests for driver registry + Codex fixture parsing |
| host/ingestion/src/protocol.ts | Re-export agent-flow protocol types for ingestion |
| host/ingestion/src/observation-store.ts | ObservationStore implementation + query helpers |
| host/ingestion/src/jsonl-tail.ts | VS Code–free JSONL tailing source |
| host/ingestion/src/ingestion-adapter.ts | Driver → store → optional IPC emission adapter |
| host/ingestion/src/index.ts | Public exports for ingestion package |
| host/ingestion/src/harness-driver.ts | HarnessDriver interface + registry |
| host/ingestion/src/drivers/index.ts | Default registry (Claude first, Codex stub) |
| host/ingestion/src/drivers/codex.ts | Codex stub driver using CodexRolloutParser |
| host/ingestion/src/drivers/claude-code.ts | Claude driver (agent-events + transcript modes) |
| host/ingestion/README.md | Package docs + root script coordination |
| host/ingestion/package.json | New package manifest (vitest/typescript) |
| host/ingestion/package-lock.json | New lockfile for ingestion package |
| host/ingestion/fixtures/simulate-events.jsonl | Fixture fallback event stream |
| electron/tsconfig.json | Switch to typecheck-only + broaden include set |
| electron/preload.ts | Expose queryRecent via contextBridge |
| electron/main.ts | Create/start ingestion host; add IPC handler |
| electron/ipc-channels.ts | Add OBSERVATION_QUERY_RECENT IPC constant |
| electron/ingestion-host.ts | Transcript discovery + fixture fallback + event forwarding |
| electron/build.mjs | esbuild bundling of Electron main/preload |
Files not reviewed (1)
- host/ingestion/package-lock.json: Generated file
Comments suppressed due to low confidence (1)
electron/ingestion-host.ts:232
- In fixture replay, this code mutates StoredObservation.event.sessionId in-place and calls getAll() twice. Since ObservationQuery is intended as a read-only surface, avoid mutating stored observations and just stamp session IDs on the outgoing event batch using stampSession().
for (const obs of this.adapter.query.getAll()) {
if (!obs.event.sessionId) {
obs.event.sessionId = planned.sessionId
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function parseAgentEventLine(line: string): AgentEvent | null { | ||
| try { | ||
| const parsed = JSON.parse(line.trim()) as Partial<AgentEvent> | ||
| if (parsed && typeof parsed.type === 'string' && typeof parsed.time === 'number') { | ||
| return parsed as AgentEvent | ||
| } | ||
| return null | ||
| } catch { | ||
| return null | ||
| } | ||
| } |
| const source = new JsonlTailSource(options.filePath, parseAgentEventLine) | ||
| this.tail = source | ||
| source.subscribe((event) => this.ingestEvent(event)) | ||
| if (options.replay) source.replayOnce() | ||
| else source.start() |
| queryRecent(n = 50): StoredObservation[] { | ||
| return this.adapter.query.recent(n) | ||
| } |
| if (!fs.existsSync(this.filePath)) { | ||
| fs.mkdirSync(path.dirname(this.filePath), { recursive: true }) | ||
| fs.writeFileSync(this.filePath, '') | ||
| } | ||
|
|
||
| const stat = fs.statSync(this.filePath) | ||
| this.fileSize = 0 | ||
| this.tail = '' | ||
| this.readFrom(0) | ||
| this.fileSize = stat.size | ||
|
|
Code Review SummaryStatus: 10 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (27 files)
Fix these issues in Kilo Cloud Previous Review SummaryCurrent summary above is authoritative. Previous snapshots are kept for context only. Previous reviewStatus: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (22 files)
Reviewed by ling-3.0-flash:free · Input: 347.7K · Output: 66.8K · Cached: 2.8M |
M2 prep for curator lookbacks: ring-buffered agent-flow events with query helpers, Claude/Codex drivers wrapping extension parsers, and an IPC-injectable ingestion adapter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Start HarnessDriver on renderer ready (Claude transcript or fixture), forward agent-event messages over the vscode-bridge IPC shape, and bundle host/ingestion into dist-electron via esbuild. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…ests - Add host/ingestion/fixtures/claude-transcript-sample.jsonl: a 15-line realistic Claude Code session transcript with all the real record types (mode, system, user, assistant with thinking/text/tool_use blocks, tool_result, ai-title, last-prompt). Synthetic content but structurally faithful to ~/.claude/projects/.../<session>.jsonl. Replaces the 18-line synthetic AgentEvent fixture as the parser test source. - Add host/ingestion/tests/transcript-parser.test.ts: exercises the REAL TranscriptParser (compiled from extension/src/) via the claude-code driver in transcript mode. Asserts the parser emits the AgentEvent vocabulary (message/agent_spawn, tool_call_start for tool_use blocks), stamps session ids, and handles negative paths (missing file -> zero events, empty file, malformed JSON line -> no crash). Plus fixture sanity checks (every line valid JSON, expected record types present). This is the test the prior suite was missing: the ingestion pivot (raw transcript -> AgentEvent) was never exercised against realistic transcript shape — only against the 18-line synthetic AgentEvent fixture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
586f9b3 to
f81144c
Compare
There was a problem hiding this comment.
[WARNING]: forwardToRenderer left as true if driver start fails
In startPlanned(), this.forwardToRenderer = true is set at line 209 before this.adapter.startDriver() is called at line 210. If startDriver() throws (e.g., the Claude transcript file becomes unavailable between discovery and driver startup), the error propagates up to the catch handler in main.ts, which sends fallback config and disconnected status. However, forwardToRenderer is never reset to false, leaving the adapter in an inconsistent state where IPC emissions are forwarded to the renderer even though no active ingestion is producing events. Reset forwardToRenderer in the error path or use a try/finally to ensure cleanup.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| private async startPlanned(planned: IngestionHostStatus): Promise<void> { | ||
| if (planned.kind === 'claude-transcript') { | ||
| console.log(`[ingestion-host] Claude transcript: ${planned.filePath}`) | ||
| this.forwardToRenderer = true |
There was a problem hiding this comment.
[WARNING]: forwardToRenderer left as true if driver start fails
In startPlanned(), this.forwardToRenderer = true is set before this.adapter.startDriver() is called. If startDriver() throws (e.g., the Claude transcript file becomes unavailable between discovery and driver startup), the error propagates up to the catch handler in main.ts, which sends fallback config and disconnected status. However, forwardToRenderer is never reset to false, leaving the adapter in an inconsistent state where IPC emissions are forwarded to the renderer even though no active ingestion is producing events. Reset forwardToRenderer in the error path or use a try/finally to ensure cleanup.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
[WARNING]: Fixture replay mutates stored observations directly
In startPlanned(), the fixture replay path at lines 228-232 mutates obs.event.sessionId on each StoredObservation returned by getAll(). Since getAll() returns a shallow copy of the buffer (the StoredObservation objects themselves are still references into the store), this assignment mutates the event object retained in the ObservationStore. Subsequent queries (including getAll() itself) will observe the stamped sessionId, which is a side effect on the store's data that bypasses the normal ingestion pipeline. Consider stamping sessionId at ingestion time (as the Codex driver does in its emit() method) rather than mutating stored observations after the fact.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| sessionId: planned.sessionId, | ||
| replay: true, | ||
| }) | ||
| for (const obs of this.adapter.query.getAll()) { |
There was a problem hiding this comment.
[WARNING]: Fixture replay mutates stored observations directly
In startPlanned(), the fixture replay path at lines 228-232 mutates obs.event.sessionId on each StoredObservation returned by getAll(). Since getAll() returns a shallow copy of the buffer (the StoredObservation objects themselves are still references into the store), this assignment mutates the event object retained in the ObservationStore. Subsequent queries (including getAll() itself) will observe the stamped sessionId, which is a side effect on the store's data that bypasses the normal ingestion pipeline. Consider stamping sessionId at ingestion time (as the Codex driver does in its emit() method) rather than mutating stored observations after the fact.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
User description
Summary
host/ingestionObservationStore + HarnessDriver registry (Claude Code + Codex stub)~/.claude/projects/**/*.jsonl; fall back to simulate-events fixtureHonest status
Test plan
npm test(ingestion package, 7 tests)npm run buildshowMockData: falsewhen transcript/fixture is availableCodeAnt-AI Description
Connect Electron to live agent transcripts and searchable observation history
What Changed
Impact
✅ Live Claude Code activity in Electron✅ Session data remains available for recent lookbacks✅ Fewer empty visualizers when no transcript is found💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.