feat(tracing): Langfuse prompt/reply/cost + one-trace-per-turn (repairs #4399 build break)#4434
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 15 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds a new ChangesTurnContent Capture and Tracing
Orchestration Checkpointer Swap
Builtin Agent Graph Wiring
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant run_turn_via_tinyagents_session
participant SpanCollector
participant LangfuseExporter
participant DownstreamConsumers
run_turn_via_tinyagents_session->>SpanCollector: AgentProgress::TurnContent { input, output }
SpanCollector->>SpanCollector: attach input/output to root turn span
run_turn_via_tinyagents_session->>DownstreamConsumers: AgentProgress::TurnContent
DownstreamConsumers->>DownstreamConsumers: ignore / skip logging
SpanCollector->>LangfuseExporter: push_spans(capture_content)
alt capture_content = true
LangfuseExporter->>LangfuseExporter: include input/output in batch
else capture_content = false
LangfuseExporter->>LangfuseExporter: withhold input/output
end
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
coderabbitai
Bot
added
bug
rust-core
![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: 26901cbba0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// (via `AgentProgress::TurnContent`); the exporter still gates transmission | ||
| /// behind `observability.agent_tracing.capture_content`. | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub input: Option<serde_json::Value>, |
There was a problem hiding this comment.
When local tracing is enabled but observability.agent_tracing.capture_content remains at its default false, SpanCollector still fills this field from TurnContent and export_spans serializes TraceSpan directly via spans_to_ndjson, so prompts are written to the configured NDJSON file or app log despite the opt-in being off. The Langfuse push path strips content, but the local exporter needs the same gate before serializing input/output.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/agent/progress_tracing.rs (1)
590-646: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate "lazily resolve turn span index" boilerplate.
TurnCostUpdatedandTurnContentboth repeat the samematch self.turn_span_index { Some(idx) => idx, None => { self.ensure_turn_span(...); self.turn_span_index.expect(...) } }pattern. Consider extracting a small helper (e.g.fn turn_span_index(&mut self, now_unix_ms: u64) -> usize) to avoid the two copies drifting.♻️ Suggested helper
+ fn turn_span_index_mut(&mut self, now_unix_ms: u64) -> usize { + match self.turn_span_index { + Some(idx) => idx, + None => { + self.ensure_turn_span(now_unix_ms); + self.turn_span_index.expect("turn span just created") + } + } + }Then both
TurnCostUpdatedandTurnContentarms calllet index = self.turn_span_index_mut(now_unix_ms);.🤖 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/openhuman/agent/progress_tracing.rs` around lines 590 - 646, The `TurnCostUpdated` and `TurnContent` branches in `progress_tracing.rs` duplicate the same lazy turn-span lookup logic. Extract that `match self.turn_span_index { ... }` pattern into a small helper on the same type, such as a `turn_span_index(now_unix_ms)` method that ensures the span exists and returns the index, then update both `AgentProgress::TurnCostUpdated` and `AgentProgress::TurnContent` to call the helper so the behavior stays in one place.
🤖 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.
Nitpick comments:
In `@src/openhuman/agent/progress_tracing.rs`:
- Around line 590-646: The `TurnCostUpdated` and `TurnContent` branches in
`progress_tracing.rs` duplicate the same lazy turn-span lookup logic. Extract
that `match self.turn_span_index { ... }` pattern into a small helper on the
same type, such as a `turn_span_index(now_unix_ms)` method that ensures the span
exists and returns the index, then update both `AgentProgress::TurnCostUpdated`
and `AgentProgress::TurnContent` to call the helper so the behavior stays in one
place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6806e69f-d052-47eb-b0a0-ffcceaa0f10a
📒 Files selected for processing (14)
src/openhuman/agent/harness/session/turn/core.rssrc/openhuman/agent/progress.rssrc/openhuman/agent/progress_tracing.rssrc/openhuman/agent/progress_tracing/langfuse.rssrc/openhuman/agent/progress_tracing/tests.rssrc/openhuman/agent_registry/agents/loader.rssrc/openhuman/channels/providers/web/progress_bridge.rssrc/openhuman/config/schema/load/env_overlay.rssrc/openhuman/config/schema/observability.rssrc/openhuman/orchestration/graph/mod.rssrc/openhuman/orchestration/graph/state.rssrc/openhuman/orchestration/ops.rssrc/openhuman/threads/turn_state/mirror.rssrc/openhuman/workflows/run_log.rs
coderabbitai
Bot
added
the
agent
M3gA-Mind
force-pushed
the
feat/from-upstream-main
branch
from
634be88 to
19f9098
Compare
|
Split out the build-fix: the |
Adds opt-in content capture and promotes token/cost onto agent trace spans, plus two correctness fixes to the Langfuse exporter path. - Content (opt-in): the turn's prompt and the model's reply ride a new AgentProgress::TurnContent event onto the turn span; the exporter emits them as Langfuse `input`/`output` only when observability.agent_tracing.capture_content is set (default off; env toggle OPENHUMAN_AGENT_TRACING_CAPTURE_CONTENT). Preserves the metadata-only, PII-free default. - Native usage/cost: the turn's gen_ai.usage.* attributes are promoted into Langfuse's native generation / usageDetails / costDetails (they were buried in metadata, so cost always rendered 0). - Fix (span-id collision): span ids were a per-turn sequence (0000...0001) reset each turn; Langfuse dedupes observations by id globally, so later turns' observations silently bound to whichever trace first claimed each id (turns appeared empty). Ids now carry a per-turn nonce prefix. - One trace per turn: the trace id is unique per request; the thread id rides along as Langfuse `sessionId` (and client as `userId`) so a conversation's per-turn traces still group under one session. - Robustness: push_spans now inspects the 207 response body and warns on per-event rejections instead of swallowing them. Tests: 29 progress_tracing tests pass, including a new exporter test asserting usage->generation promotion and the content gate (on/off).
…ntent toggle Adds targeted unit tests for the new tracing behavior to lift diff coverage on the changed lines: - collector: TurnContent attaches input/output to the turn span; session_group is stamped as thread.id and client as user.id; span ids are globally unique across turns (regression guard for the Langfuse dedup collision). - exporter: trace-create promotes user.id/thread.id to Langfuse userId/sessionId. - run_log: TurnContent produces no log line (content stays out of the run log). - config: OPENHUMAN_AGENT_TRACING_CAPTURE_CONTENT env toggle flips the flag on/off. All six pass (cargo test --lib).
M3gA-Mind
force-pushed
the
feat/from-upstream-main
branch
from
19f9098 to
ab66c7f
Compare
…dit match The tracing feature adds the AgentProgress::TurnContent variant; the harness-subagent-audit bin exhaustively matches AgentProgress and didn't cover it, so `cargo check --all-targets` (CI's Rust Quality clippy) failed E0004 even though `cargo check --lib` passed (it doesn't build bins). Fold TurnContent into the ignored-variants arm — an audit tool has no use for prompt/reply content.
2 participants
Summary
input/output— opt-in viaobservability.agent_tracing.capture_content(default off, preserves the PII-free default).usageDetails/costDetails(previously buried in span metadata, so cost always rendered0).0000…0001) reset each turn, and Langfuse dedupes observations by id globally, so later turns' observations silently bound to the first trace that claimed each id (new traces looked empty). Ids now carry a per-turn nonce.sessionId(thread) +userId(client).push_spansnow surfaces207per-event rejections instead of swallowing them.Problem
Agent traces in Langfuse were unusable: prompt/reply were never sent, cost always showed
0(token/cost rode in span metadata, not Langfuse's native fields), and the observation-id collision above made observations from any turn after the first vanish from their trace.Solution
AgentProgress::TurnContentcarries the prompt/reply onto the turn span; the exporter emits them as Langfuseinput/outputonly whenobservability.agent_tracing.capture_contentis set (env toggleOPENHUMAN_AGENT_TRACING_CAPTURE_CONTENT), preserving the metadata-only default.gen_ai.usage.*attributes are promoted onto a LangfusegenerationwithusageDetails/costDetails/model.sessionIdfor grouping.push_spansinspects the207body and warns on per-event rejections.Submission Checklist
progress_tracing::langfuseexporter test (usage→generation promotion + content on/off gate) plus collector tests (TurnContent, grouping, span-id uniqueness), run_log skip, and the env-toggle test. 29+ tests green.diff-coverjob is the authoritative gate and re-runs on this rebase.Impact
AI Authored PR Metadata
Validation Run
format:check/typecheck— no frontend changes.cargo test --lib progress_tracing→ all green (incl. new exporter test).cargo fmtclean;GGML_NATIVE=OFF cargo checkgreen.Behavior Changes
capture_contentis enabled; one Langfuse trace per turn grouped by session/user.