feat(claw): add session usage analytics#1969
Conversation
|
Claude (@claude) review |
|
Greptile (@greptileai) review |
|
Claude finished Nikhil (@shadowfax92)'s task in 2m 53s —— View job Code Review
Overall this is a clean, well-structured implementation. The design is disciplined: the tracker is pure in-memory state with no analytics coupling, the emit boundary is a single point ( Strengths
Observations1.
|
Greptile SummaryThis PR adds aggregate session usage analytics to the Claw server. The main changes are:
Confidence Score: 4/5The session teardown path can emit incomplete usage and should be fixed before merging.
packages/browseros-agent/apps/claw-server-rust/src/services/sessions/manager.rs Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Call as Tool call
participant Usage as Usage tracker
participant Manager as Session manager
participant Analytics as Analytics sink
Call->>Call: Dispatch tool
Manager->>Call: Request cancellation
Manager->>Usage: Take usage snapshot
Manager->>Analytics: Emit tool and session events
Call->>Usage: Record completed call
Note over Usage,Analytics: Late record is absent from emitted events
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Call as Tool call
participant Usage as Usage tracker
participant Manager as Session manager
participant Analytics as Analytics sink
Call->>Call: Dispatch tool
Manager->>Call: Request cancellation
Manager->>Usage: Take usage snapshot
Manager->>Analytics: Emit tool and session events
Call->>Usage: Record completed call
Note over Usage,Analytics: Late record is absent from emitted events
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/browseros-agent/apps/claw-server-rust/src/services/sessions/manager.rs:393
**Teardown Snapshots In-Flight Usage**
When a session closes during a tool call, `cancel_active_dispatches()` requests cancellation but does not wait for the dispatch to finish. This snapshot can therefore run before `finish_tool_call()` records the completed or cancelled call, permanently omitting it from both the per-tool summary and the terminal dispatch count.
Reviews (1): Last reviewed commit: "feat(claw): emit session usage summaries" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| kind: &str, | ||
| audit_result: AppResult<()>, | ||
| ) -> AppResult<()> { | ||
| let usage = session.usage_snapshot().await; |
There was a problem hiding this comment.
When a session closes during a tool call, cancel_active_dispatches() requests cancellation but does not wait for the dispatch to finish. This snapshot can therefore run before finish_tool_call() records the completed or cancelled call, permanently omitting it from both the per-tool summary and the terminal dispatch count.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/claw-server-rust/src/services/sessions/manager.rs
Line: 393
Comment:
**Teardown Snapshots In-Flight Usage**
When a session closes during a tool call, `cancel_active_dispatches()` requests cancellation but does not wait for the dispatch to finish. This snapshot can therefore run before `finish_tool_call()` records the completed or cancelled call, permanently omitting it from both the per-tool summary and the terminal dispatch count.
How can I resolve this? If you propose a fix, please make it concise.
Greptile SummaryThis PR adds aggregate session usage analytics to BrowserClaw. The main changes are:
Confidence Score: 4/5The usage lifecycle and concurrency metric need fixes before merging.
packages/browseros-agent/apps/claw-server-rust/src/api/mcp/service.rs; packages/browseros-agent/apps/claw-server-rust/src/services/sessions/manager.rs Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Call as Tool call
participant Sessions as Session manager
participant Usage as Usage tracker
participant Analytics as Analytics sink
Call->>Usage: mark session used
Call->>Sessions: count live ever-used sessions
Sessions-->>Call: sampled count
Call->>Call: dispatch tool
par Teardown may start
Sessions->>Usage: snapshot usage
Sessions->>Analytics: emit terminal events
and Call completes
Call->>Usage: record dispatch and duration
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Call as Tool call
participant Sessions as Session manager
participant Usage as Usage tracker
participant Analytics as Analytics sink
Call->>Usage: mark session used
Call->>Sessions: count live ever-used sessions
Sessions-->>Call: sampled count
Call->>Call: dispatch tool
par Teardown may start
Sessions->>Usage: snapshot usage
Sessions->>Analytics: emit terminal events
and Call completes
Call->>Usage: record dispatch and duration
end
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/browseros-agent/apps/claw-server-rust/src/services/sessions/manager.rs:393
**Teardown Snapshots Before Call Completion**
When an idle sweep or transport close removes a session during a tool call, teardown can take this snapshot before `finish_tool_call` records the cancelled call. The terminal events then omit that dispatch and its duration, while the later update remains only on the detached session and is never emitted.
### Issue 2 of 2
packages/browseros-agent/apps/claw-server-rust/src/api/mcp/service.rs:327
**Concurrency Counts Idle Used Sessions**
`used_count()` counts every live session that has ever entered a tool call, not sessions with calls currently in flight. Two long-lived sessions making sequential calls therefore record a maximum concurrency of two even though their calls never overlap, inflating the new analytics field.
Reviews (2): Last reviewed commit: "feat(claw): emit session usage summaries" | Re-trigger Greptile |
| kind: &str, | ||
| audit_result: AppResult<()>, | ||
| ) -> AppResult<()> { | ||
| let usage = session.usage_snapshot().await; |
There was a problem hiding this comment.
When an idle sweep or transport close removes a session during a tool call, teardown can take this snapshot before finish_tool_call records the cancelled call. The terminal events then omit that dispatch and its duration, while the later update remains only on the detached session and is never emitted.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/claw-server-rust/src/services/sessions/manager.rs
Line: 393
Comment:
**Teardown Snapshots Before Call Completion**
When an idle sweep or transport close removes a session during a tool call, teardown can take this snapshot before `finish_tool_call` records the cancelled call. The terminal events then omit that dispatch and its duration, while the later update remains only on the detached session and is never emitted.
How can I resolve this? If you propose a fix, please make it concise.| ownership_key, | ||
| agent_label: started.agent_label, | ||
| started.session.mark_used(); | ||
| let concurrent_used_sessions = self.state.sessions.used_count().await.max(1); |
There was a problem hiding this comment.
used_count() counts every live session that has ever entered a tool call, not sessions with calls currently in flight. Two long-lived sessions making sequential calls therefore record a maximum concurrency of two even though their calls never overlap, inflating the new analytics field.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/claw-server-rust/src/api/mcp/service.rs
Line: 327
Comment:
**Concurrency Counts Idle Used Sessions**
`used_count()` counts every live session that has ever entered a tool call, not sessions with calls currently in flight. Two long-lived sessions making sequential calls therefore record a maximum concurrency of two even though their calls never overlap, inflating the new analytics field.
How can I resolve this? If you propose a fix, please make it concise.
✅ Tests passed — 1607/1611
|
Javier Guerrero (javi0guerrero4-cloud)
left a comment
Javier Guerrero (javi0guerrero4-cloud)
left a comment
There was a problem hiding this comment.
Ok ready
2 participants
Summary
Design
A session-owned in-memory tracker records each recognized
tools/callonce at the common completion boundary. Exactly-once teardown snapshots deterministic aggregates, emits sorted per-tool summaries, then emits the enriched terminal session event; no per-dispatch analytics, identifiers, content, or persistence are added.Test plan
cargo fmt --all -- --checkcargo test -p claw-server-rust --lockedcargo clippy -p claw-server-rust --all-targets --locked -- -D warningscargo test --workspace --lockedbun run test:claw-api-contract