feat(traces): record model, reasoning effort, and token usage per session event#320
Open
efenocchi wants to merge 13 commits into
Open
feat(traces): record model, reasoning effort, and token usage per session event#320efenocchi wants to merge 13 commits into
efenocchi wants to merge 13 commits into
Conversation
New shared module src/notifications/model-usage.ts with two best-effort parsers that read the on-disk transcript the capture hooks already receive a path to: - parseClaudeTurnMeta: last assistant turn's model + usage from a Claude Code transcript. Reasoning effort left null (no per-message field). - parseCodexTurnMeta: model + reasoning_effort (turn_context) + per-turn and cumulative token usage (token_count) from a Codex rollout. Both normalize onto a shared NormalizedUsage shape (cache_read_tokens, cache_creation_tokens, reasoning_output_tokens, total_tokens) so a per-model rollup is a single GROUP BY over the sessions table. Absent fields stay absent; any read/parse failure returns null. Covered by tests/shared/notifications-model-usage.test.ts (real transcript shapes: reverse-scan, omitted fields, malformed lines, model fallback, last-vs-total usage).
Enrich the assistant_message trace row (Stop event) with model, reasoning_effort (null for Claude) and per-turn token_usage, read from the transcript's last assistant turn via parseClaudeTurnMeta. Best-effort: the row is written unchanged when the transcript is unreadable.
Enrich the Codex trace meta with reasoning_effort, per-turn token_usage and cumulative token_usage_total from the rollout (parseCodexTurnMeta). Codex emits no assistant event, so the enrichment rides on every user/tool row; the running total rolls up per model with MAX, not SUM. The turn_context model refines the payload model when present.
Four correctness fixes from the codex review of this branch: - SubagentStop: parse agent_transcript_path when present. transcript_path points at the parent session, so the subagent's assistant event was recording the parent model/usage (capture.ts). - Codex per-turn usage: clear it on each turn_context so a new turn's model never inherits the previous turn's tokens. total stays cumulative. - token_usage_total: documented as whole-session cumulative (across all models), not per-model; per-model totals sum per-turn usage deduped by turn_id. Fixed the misleading MAX-per-model note. - Best-effort contract: skip non-object JSONL roots (a bare `null` line) before dereferencing, in both parsers. Tests extended: turn-boundary model change omits stale usage; bare-null line survives in both parsers.
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds best-effort model and token-usage extraction for Claude, Codex, Pi, and OpenClaw captures, validates the metadata through unit and E2E tests, and prevents recognized provider model identifiers from being redacted as secrets. ChangesTranscript metadata enrichment
End-to-end usage verification
Model identifier redaction handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CaptureHooks
participant ModelUsageParsers
participant TranscriptFiles
participant SessionsTable
CaptureHooks->>ModelUsageParsers: Parse transcript or SDK usage metadata
ModelUsageParsers->>TranscriptFiles: Read JSONL transcript when needed
TranscriptFiles-->>ModelUsageParsers: Return transcript events
ModelUsageParsers-->>CaptureHooks: Return normalized model metadata
CaptureHooks->>SessionsTable: Insert enriched capture rows
SessionsTable-->>CaptureHooks: Return stored entries
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
Contributor
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 4 files changed
Generated for commit 675a7c4. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/notifications/model-usage.ts`:
- Around line 72-78: Update readLines to avoid synchronously rereading and
splitting the entire transcript on every event. Maintain a per-transcript
cursor/cache or otherwise read only bytes appended since the previous call,
while preserving the missing-transcript handling and returning the newly
available lines needed by callers.
- Around line 62-64: Update toNum in model-usage.ts to return a value only when
v is a finite, non-negative safe integer; return undefined for negative,
fractional, unsafe, or otherwise invalid inputs before they reach usage
aggregation persistence.
- Around line 204-211: Update the turn_context handling in the model-usage
parser to reset model and reasoningEffort for every new turn, using the current
hook model only as the fallback when the new context’s model is missing or
malformed and clearing invalid or absent effort instead of retaining the
previous value. Preserve last/total behavior, and add a regression test covering
a valid first context followed by a context with missing or malformed
model/effort.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7de3d604-2726-4324-b4f3-4cebcf231d7a
📒 Files selected for processing (4)
src/hooks/capture.tssrc/hooks/codex/capture.tssrc/notifications/model-usage.tstests/shared/notifications-model-usage.test.ts
Comment on lines
+72
to
+78
| function readLines(path: string): string[] | null { | ||
| if (!path || !existsSync(path)) { | ||
| log(`transcript missing: ${path}`); | ||
| return null; | ||
| } | ||
| try { | ||
| return readFileSync(path, "utf-8").split("\n"); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Avoid synchronously rereading the entire transcript for every event.
Codex calls this on every user/tool row, making transcript processing cumulative O(n²) and blocking the hook as sessions grow. Read only the tail/new bytes, or maintain a per-transcript cursor/cache.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/notifications/model-usage.ts` around lines 72 - 78, Update readLines to
avoid synchronously rereading and splitting the entire transcript on every
event. Maintain a per-transcript cursor/cache or otherwise read only bytes
appended since the previous call, while preserving the missing-transcript
handling and returning the newly available lines needed by callers.
…effort per turn - toNum: accept only non-negative safe integers, so negative / fractional / unsafe values never enter token aggregates. - turn_context: reset both model and reasoning effort per turn (hook model as fallback) so a later turn that omits effort no longer inherits the previous turn's effort — same scoping fix already applied to per-turn usage. Tests: per-turn model/effort reset with hook-model fallback; negative and fractional token counts dropped.
Collaborator
Author
|
CodeRabbit triage:
|
normalizeSdkUsage maps the pi/openclaw usage shape
{input, output, cacheRead, cacheWrite, totalTokens} onto the shared
NormalizedUsage keys; sdkTurnMeta builds the {model, token_usage} enrichment
from an in-process SDK message (these runtimes expose no reasoning effort).
Same non-negative-safe-integer filtering as the transcript parsers.
Tests cover the real Pi and OpenClaw usage shapes (incl. cacheWrite ->
cache_creation_tokens) and the model-only / empty cases.
Storing the model on every trace row exposed a latent false positive: the high-entropy backstop masked long dated slugs like `claude-haiku-4-5-20251001` -> `********`, which would silently break per-model token rollups. Add a model-identifier guard to looksLikeSecret (claude/gpt/o*/gemini/... optionally provider-prefixed) so these survive redaction. Regression test asserts common ids are preserved.
The agent_end auto-capture loop now enriches assistant rows with model and normalized token_usage from the SDK message (via the shared sdkTurnMeta). User rows are unchanged.
message_end now enriches the assistant row with model and normalized token_usage from the SDK message. Pi ships as raw .ts with no shared-module imports, so the normalizer is inlined in lockstep with src/notifications/model-usage.ts (same pattern as the inlined JWT helpers).
scripts/trace-model-usage-e2e.mjs drives the real capture hooks (Claude Code, Codex, Cursor, Hermes) against every model found in the local transcripts, writing to a throwaway table and reading the rows back to show what landed in the JSONB message column. Pi and OpenClaw capture in-process (not via stdin hooks), so it proves their enriched entry from a real transcript message via the shared sdkTurnMeta. Suppresses side effects (HIVEMIND_WIKI_WORKER=1), disables embeddings, and cleans up its rows.
Address codex review [P1]: the earlier exemption used a case-insensitive prefix match with an open charset, so a random key wearing a model prefix (`gpt-AbCd…30chars`) would be exempted and stored unmasked. Narrow it to lowercase-only ids with each dot/dash segment capped at 12 chars, so no 24+ high-entropy run (mixed-case or long) can satisfy it. Added a negative test asserting prefixed high-entropy secrets are still masked.
… agent Address codex review [P2] plus the sessions-table write behaviour observed while running it: - The hooks swallow insert errors and exit 0, so exit code is not proof. Read the rows back and assert each agent's capture path landed rows, with token_usage present for the token-bearing agents (claude_code, codex). - The table drops rapid same-table bursts (200 OK, rows=0, row never lands) and is read-after-write lagged. Pace inserts (SLEEP_MS), poll the read-back, and retry stragglers; report invoked-vs-visible per agent instead of silently treating a hidden tail as success. - Derive the agent from the session_id prefix (the SQL `agent` column reads back unreliably here).
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/trace-model-usage-e2e.mjs`:
- Around line 290-301: Strengthen scripts/trace-model-usage-e2e.mjs at lines
290-301 by retaining expected parser metadata per session and comparing every
stored model, reasoning-effort, usage, and cumulative-usage field for each
capture path, rather than checking only row presence; update lines 312-318 to
exercise Pi’s inline normalizer and both integrations’ real entry/write paths
with assertions, ensuring logging or missing transcripts cannot count as
success.
- Around line 297-301: Update the validation loops in the trace-model usage
check to require rows only for agents recorded in invokedByAgent. Gate both
modelsByAgent and tokenRowsByAgent checks on whether each agent was actually
invoked, while preserving the existing missing-row problems for invoked agents.
In `@src/hooks/shared/redact.ts`:
- Around line 70-74: Update the model-identifier whitelist regex in the
redaction logic to recognize un-hyphenated version digits for the affected
families, such as optional single digits after llama and qwen, while preserving
existing matching behavior. Extend the accepted prefix alternatives to include
common Bedrock/provider prefixes—anthropic, amazon, cohere, meta, us, and eu—so
provider-qualified identifiers remain unredacted after colon splitting.
In `@tests/shared/redact.test.ts`:
- Around line 223-251: Replace the generic toContain assertions in the
provider-model and prefixed-secret tests with exact toBe assertions against the
complete expected redacted or unchanged payload. Keep redactSecrets and MASK
behavior unchanged, and ensure each assertion verifies the entire JSON or token
string rather than only a substring.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0aba9947-0d09-4f5b-a85f-cf481a1a56e1
📒 Files selected for processing (7)
harnesses/openclaw/src/index.tsharnesses/pi/extension-source/hivemind.tsscripts/trace-model-usage-e2e.mjssrc/hooks/shared/redact.tssrc/notifications/model-usage.tstests/shared/notifications-model-usage.test.tstests/shared/redact.test.ts
Comment on lines
+290
to
+301
| // Guard: exit code 0 from a capture hook is not proof — verify each agent's | ||
| // capture path actually wrote correct rows. We assert per-agent presence | ||
| // (and token_usage for the token-bearing agents) rather than requiring every | ||
| // one of a 20-model burst to be visible, which the backend's read-lag won't | ||
| // reliably surface. A genuinely broken hook lands zero rows for its agent. | ||
| const problems = []; | ||
| if (rows.length === 0) problems.push("no rows landed at all"); | ||
| for (const agent of ["claude_code", "codex", "cursor"]) { | ||
| if (!(modelsByAgent.get(agent)?.size)) problems.push(`no ${agent} rows landed`); | ||
| } | ||
| for (const agent of ["claude_code", "codex"]) { | ||
| if (!tokenRowsByAgent.get(agent)) problems.push(`no ${agent} row carried token_usage`); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make the harness validate actual metadata across every capture path.
The current checks establish partial presence, not correctness or Pi/OpenClaw integration behavior.
scripts/trace-model-usage-e2e.mjs#L290-L301: retain expected parser metadata per session and compare every stored model, reasoning-effort, usage, and cumulative-usage field.scripts/trace-model-usage-e2e.mjs#L312-L318: exercise Pi’s inline normalizer and both integrations’ real entry/write paths with assertions; do not treat logging or absent transcripts as success.
📍 Affects 1 file
scripts/trace-model-usage-e2e.mjs#L290-L301(this comment)scripts/trace-model-usage-e2e.mjs#L312-L318
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/trace-model-usage-e2e.mjs` around lines 290 - 301, Strengthen
scripts/trace-model-usage-e2e.mjs at lines 290-301 by retaining expected parser
metadata per session and comparing every stored model, reasoning-effort, usage,
and cumulative-usage field for each capture path, rather than checking only row
presence; update lines 312-318 to exercise Pi’s inline normalizer and both
integrations’ real entry/write paths with assertions, ensuring logging or
missing transcripts cannot count as success.
- redact: broaden the model-id guard to cover cloud/region-prefixed slugs (us.anthropic.claude-3-5-sonnet-20241022-v2) and digit-fused families (qwen2.5-coder-32b-instruct, llama3.1-405b-instruct) so they aren't over-redacted, while keeping the lowercase + <=12-char-segment invariants that stop a secret from slipping through. Tests use exact assertions. - e2e: require only agents that were actually invoked (a CI box may lack a given agent's transcripts); assert token-bearing rows carry positive integer input+output tokens (correctness, not mere presence); and assert the Pi/OpenClaw sdkTurnMeta proof yields valid positive token_usage.
Collaborator
Author
|
CodeRabbit round-2 triage (all in
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Records model, reasoning effort, and token consumption on captured session traces, so per-model token usage is a simple query over the
sessionstable. Covers every code assistant the plugin captures.Neither Claude Code nor Codex passes this in the hook payload, but both write it to the on-disk transcript the capture hooks already receive a path to. Pi and OpenClaw expose model+usage on the in-process SDK message. A shared module reads/normalizes it and the capture paths enrich the JSONB
messagecolumn — no schema migration.Coverage (verified end-to-end against real transcripts)
Stored shape (JSONB
message){ "model": "claude-opus-4-8", "reasoning_effort": null, "token_usage": { "input_tokens": 131, "output_tokens": 403, "cache_read_tokens": 357059, "cache_creation_tokens": 2102 } }Codex rows additionally carry
reasoning_effortandtoken_usage_total(whole-session cumulative).How
src/notifications/model-usage.ts:parseClaudeTurnMeta,parseCodexTurnMeta, andnormalizeSdkUsage/sdkTurnMeta(Pi/OpenClaw). Normalized keys across agents; absent fields stay absent; non-negative-safe-integer filtering; best-effort (null on any failure).src/hooks/capture.ts(Claude),src/hooks/codex/capture.ts(Codex),harnesses/openclaw/src/index.ts(import),harnesses/pi/extension-source/hivemind.ts(inlined — pi ships raw .ts).src/hooks/shared/redact.ts: guard so the entropy backstop doesn't shred provider model ids now stored in every row (would break per-model rollups) — narrow enough that a secret wearing a model prefix is still masked.Testing
scripts/trace-model-usage-e2e.mjsdrives the real capture hooks (Claude/Codex/Cursor/Hermes) across every model in the local transcripts, writes to a throwaway table, reads back, and asserts each agent's rows landed with token_usage for the token-bearing agents. Pi/OpenClaw capture in-process, so it proves their enriched entry from a real transcript message. Passes.tests/shared/notifications-model-usage.test.ts, redaction guard tests intests/shared/redact.test.ts.tscclean; full suite green except one pre-existing, unrelatedcli-bundle-runtimetree-sitter test (environmental; my diff touches no graph/CLI code).Review
codex reviewrun twice (initial + expanded scope); all findings fixed — SubagentStop transcript, Codex turn-boundary scoping, cumulative-vs-per-model semantics, non-object JSONL guard, redaction guard tightening, and e2e capture-verification. CodeRabbit findings (invalid token counts, per-turn effort reset) also fixed.Summary by CodeRabbit