feat: add tape recording system for workspace analyze sessions#30
feat: add tape recording system for workspace analyze sessions#30
Conversation
Implement a Go port of republic's tape system that records agent execution as append-only JSONL entries. Each workspace analyze run generates a .tape.jsonl file capturing messages, tool calls, results, errors, and events with token usage metadata. - pkg/tape: entry model, JSONL store, eino callback handler, slog handler - Integrate JSONL tape recorder in analyzer via Config.TapePath - Register slog tape handler globally in main.go for live logging
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 311196d385
ℹ️ 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 config.TapePath != "" { | ||
| jsonlStore := tape.NewJSONLStore(config.TapePath) | ||
| callbacks.AppendGlobalHandlers(tape.NewHandler(jsonlStore)) | ||
| slog.Info("Tape recording enabled", "path", config.TapePath) |
There was a problem hiding this comment.
Avoid appending tape callbacks to global handler state
RunAgentWithPrompt installs a new tape callback via callbacks.AppendGlobalHandlers on every invocation, but global handlers are process-wide and never removed. If this function is called more than once in a process (e.g., repeated analyzer runs in tests or embedded usage), later runs will invoke all previously registered tape handlers, causing duplicated events and writes to stale tape files from earlier sessions.
Useful? React with 👍 / 👎.
| func NewJSONLStore(path string) *JSONLStore { | ||
| return &JSONLStore{ | ||
| path: path, | ||
| nextID: 1, |
There was a problem hiding this comment.
Preserve unique entry IDs when appending to existing tape
The store always starts nextID at 1, but writes are append-only, so rerunning workspace analyze against an existing .tape.jsonl file creates duplicate IDs in the same tape. Any consumer that relies on Entry.ID uniqueness or monotonic ordering will get ambiguous records after the first rerun.
Useful? React with 👍 / 👎.
NewJSONLStore now scans existing JSONL file for the maximum entry ID and resumes from there, preventing duplicate IDs across reruns.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 241ca19df1
ℹ️ 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".
| var lastOutput callbacks.CallbackOutput | ||
| for { | ||
| chunk, err := output.Recv() | ||
| if err != nil { | ||
| break | ||
| } | ||
| lastOutput = chunk | ||
| } |
There was a problem hiding this comment.
Aggregate streamed model chunks before writing tape
onEndWithStream overwrites lastOutput for every Recv() and only records the final chunk, so streamed chat responses lose earlier chunks (including incremental content/tool-call arguments) and the resulting tape can be incomplete or wrong for multi-chunk generations. Because Eino streaming callbacks deliver per-chunk outputs, this should either append each chunk or merge chunks before calling recordModelOutput.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| func slogOnStart(ctx context.Context, info *callbacks.RunInfo, input callbacks.CallbackInput) context.Context { | ||
| if info.Component == components.ComponentOfChatModel { |
There was a problem hiding this comment.
Guard global slog callbacks against nil RunInfo
This handler dereferences info without a nil check; if any component triggers callbacks without injecting RunInfo (which Eino allows for standalone/misconfigured components), the global handler will panic and can crash the CLI. Since NewSlogHandler is appended globally in main, add a defensive info == nil path before reading info.Component/info.Name in these callbacks.
Useful? React with 👍 / 👎.
…sit) Add nil checks for RunInfo in all tape handler and slog handler callbacks. Eino allows callbacks to be triggered without RunInfo for standalone or misconfigured components; without these guards, the global handlers would panic and crash the CLI.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 511195df4f
ℹ️ 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 maxID == 0 { | ||
| return 1 | ||
| } | ||
| return maxID + 1 |
There was a problem hiding this comment.
Check scanner errors before reusing tape IDs
scanMaxID never checks sc.Err() after the scan loop, so it treats a truncated scan as success and returns maxID+1. Because Append can write entries larger than the scanner's 10MB limit, a later reopen can stop early with ErrTooLong, miss the true highest ID, and then assign duplicate IDs on subsequent appends, corrupting tape ordering.
Useful? React with 👍 / 👎.
| if info == nil { | ||
| return | ||
| } |
There was a problem hiding this comment.
Close stream copies even when RunInfo is missing
This early return skips closing the output stream copy when info is nil, but OnEndWithStreamOutput handlers are required to close their stream copies to let the original stream fully release resources. In standalone or misconfigured callback contexts where RunInfo can be absent, this can leak goroutines and keep stream cleanup from completing (the same pattern appears in pkg/tape/slog_handler.go).
Useful? React with 👍 / 👎.
Implement a Go port of republic's tape system that records agent
execution as append-only JSONL entries. Each workspace analyze run
generates a .tape.jsonl file capturing messages, tool calls, results,
errors, and events with token usage metadata.