feat: implement Langfuse feedback scores#4505
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Langfuse score submission for chat feedback, wires a new observability RPC through core registries, propagates turn trace IDs into thread metadata, and renders thumbs up/down feedback controls with localized labels. ChangesUser feedback scoring
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Conversations
participant CoreRpc
participant SubmitScoreHandler
participant Langfuse
User->>Conversations: Click thumbs up/down
Conversations->>CoreRpc: openhuman.observability_submit_score(trace_id, name, value)
CoreRpc->>SubmitScoreHandler: handle_submit_score(params)
SubmitScoreHandler->>Langfuse: push_score(trace_id, name, value, comment)
Langfuse-->>SubmitScoreHandler: Ok/Err
SubmitScoreHandler-->>Conversations: SubmitScoreResponse{ok: true}
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
coderabbitai
Bot
added
feature
There was a problem hiding this comment.
Pull request overview
Implements Langfuse “score-create” ingestion so user feedback (thumbs up/down) can be attached to an agent-turn trace via a new core RPC method, and wires basic UI controls in the chat thread to submit those scores.
Changes:
- Added
push_scoreto the Langfuse progress tracing exporter and exposed it viaopenhuman.observability_submit_score. - Registered the new
observabilitycontroller/schema in the core controller registry. - Added thumbs up/down buttons in the Conversations UI to submit feedback scores.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/openhuman/agent/progress_tracing/rpc.rs |
New RPC controller/schema for submitting Langfuse scores. |
src/openhuman/agent/progress_tracing/langfuse.rs |
Adds push_score which sends score-create ingestion batches to Langfuse. |
src/openhuman/agent/progress_tracing.rs |
Exposes the new rpc submodule. |
src/core/all.rs |
Registers the new observability controller and namespace description. |
app/src/pages/Conversations.tsx |
Adds thumbs up/down UI to submit feedback scores for assistant messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else { | ||
| tracing::debug!( | ||
| target: LOG_TARGET, | ||
| "[agent-tracing] pushed {span_count} spans to Langfuse ({status})" | ||
| ); | ||
| Ok(()) | ||
| } |
| use serde::{Deserialize, Serialize}; | ||
| use serde_json::{Map, Value}; | ||
| use std::future::Future; | ||
| use std::pin::Pin; |
| fn handle_submit_score(params: Map<String, Value>) -> ControllerFuture { | ||
| Box::pin(async move { | ||
| let req = serde_json::from_value::<SubmitScoreRequest>(Value::Object(params)) | ||
| .map_err(|e| format!("Invalid SubmitScoreRequest: {}", e))?; | ||
|
|
||
| let config = Config::load_or_init().await.map_err(|e| e.to_string())?; | ||
| let _ = langfuse::push_score( | ||
| &config, | ||
| &req.trace_id, | ||
| &req.name, | ||
| req.value, | ||
| req.comment.as_deref(), | ||
| ) | ||
| .await; | ||
|
|
||
| Ok(serde_json::to_value(SubmitScoreResponse { ok: true }).unwrap()) | ||
| }) | ||
| } |
| void callCoreRpc('openhuman.observability_submit_score', { | ||
| trace_id: `${selectedThreadId}:${msg.id}`, | ||
| name: 'user-feedback', | ||
| value: 1.0, | ||
| }); | ||
| }} | ||
| title="Good response" | ||
| > | ||
| <LuThumbsUp className="h-3 w-3" /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| className="rounded-full bg-white p-1 text-stone-500 shadow-sm ring-1 ring-inset ring-stone-200 hover:bg-stone-50 hover:text-stone-700 dark:bg-stone-800 dark:text-stone-400 dark:ring-stone-700 dark:hover:bg-stone-700 dark:hover:text-stone-200" | ||
| onClick={() => { | ||
| void callCoreRpc('openhuman.observability_submit_score', { | ||
| trace_id: `${selectedThreadId}:${msg.id}`, | ||
| name: 'user-feedback', | ||
| value: 0.0, | ||
| }); |
| value: 1.0, | ||
| }); | ||
| }} | ||
| title="Good response" |
| value: 0.0, | ||
| }); | ||
| }} | ||
| title="Bad response" |
| title="Good response" | ||
| > | ||
| <LuThumbsUp className="h-3 w-3" /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| className="rounded-full bg-white p-1 text-stone-500 shadow-sm ring-1 ring-inset ring-stone-200 hover:bg-stone-50 hover:text-stone-700 dark:bg-stone-800 dark:text-stone-400 dark:ring-stone-700 dark:hover:bg-stone-700 dark:hover:text-stone-200" | ||
| onClick={() => { | ||
| void callCoreRpc('openhuman.observability_submit_score', { | ||
| trace_id: `${selectedThreadId}:${msg.id}`, | ||
| name: 'user-feedback', | ||
| value: 0.0, | ||
| }); | ||
| }} | ||
| title="Bad response" |
| /// Push a single `score-create` event to Langfuse, attaching it to an existing | ||
| /// trace. Follows the same privacy gate, resolution, and error handling patterns | ||
| /// as `push_spans`. | ||
| pub(crate) async fn push_score( | ||
| config: &Config, | ||
| trace_id: &str, | ||
| name: &str, | ||
| value: f64, | ||
| comment: Option<&str>, | ||
| ) -> Result<(), String> { | ||
| if !config.observability.share_usage_data { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let url = ingestion_url(config); | ||
| if !url.starts_with("http") { | ||
| return Err(format!( | ||
| "could not resolve Langfuse ingestion URL from backend host (got {url:?})" | ||
| )); | ||
| } | ||
|
|
||
| let token = require_live_session_token(config)?; | ||
| let mut body = json!({ | ||
| "id": new_event_id(), | ||
| "traceId": trace_id, | ||
| "name": name, | ||
| "value": value, | ||
| }); | ||
| if let Some(c) = comment { | ||
| body["comment"] = json!(c); | ||
| } | ||
|
|
||
| let batch = json!({ | ||
| "batch": [{ | ||
| "id": new_event_id(), | ||
| "type": "score-create", | ||
| "timestamp": iso_millis(chrono::Utc::now().timestamp_millis() as u64), | ||
| "body": body | ||
| }] | ||
| }); |
| {msg.sender === 'agent' && selectedThreadId && ( | ||
| <> | ||
| <button | ||
| type="button" | ||
| className="rounded-full bg-white p-1 text-stone-500 shadow-sm ring-1 ring-inset ring-stone-200 hover:bg-stone-50 hover:text-stone-700 dark:bg-stone-800 dark:text-stone-400 dark:ring-stone-700 dark:hover:bg-stone-700 dark:hover:text-stone-200" | ||
| onClick={() => { | ||
| void callCoreRpc('openhuman.observability_submit_score', { | ||
| trace_id: `${selectedThreadId}:${msg.id}`, | ||
| name: 'user-feedback', | ||
| value: 1.0, | ||
| }); | ||
| }} | ||
| title="Good response" | ||
| > | ||
| <LuThumbsUp className="h-3 w-3" /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| className="rounded-full bg-white p-1 text-stone-500 shadow-sm ring-1 ring-inset ring-stone-200 hover:bg-stone-50 hover:text-stone-700 dark:bg-stone-800 dark:text-stone-400 dark:ring-stone-700 dark:hover:bg-stone-700 dark:hover:text-stone-200" | ||
| onClick={() => { | ||
| void callCoreRpc('openhuman.observability_submit_score', { | ||
| trace_id: `${selectedThreadId}:${msg.id}`, | ||
| name: 'user-feedback', | ||
| value: 0.0, | ||
| }); | ||
| }} | ||
| title="Bad response" | ||
| > | ||
| <LuThumbsDown className="h-3 w-3" /> | ||
| </button> | ||
| </> |
![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
Line 129 in 0456897
Keep feedback RPC out of agent-facing discovery
Registering this score-submission endpoint in the global controller registry also puts it in the agent-facing schema catalog/tool listings; the same file reserves the internal registry for renderer-callable methods that should not be advertised to agents. In any run where agents can see controller-backed tools, they can now fabricate user-feedback scores for arbitrary traces, which undermines the observability data this endpoint is meant to collect from the UI.
ℹ️ 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".
| target: LOG_TARGET, | ||
| "[agent-tracing] pushed {span_count} spans to Langfuse ({status})" | ||
| ); | ||
| Ok(()) |
There was a problem hiding this comment.
This Ok(()) is still inside the else branch and there is no closing brace for push_spans before the new push_score item starts, so langfuse.rs does not parse/compile once this module is built. Close the else block and the function before adding the new helper.
Useful? React with 👍 / 👎.
| FieldSchema { | ||
| name: "trace_id", | ||
| comment: "Trace ID to attach the score to.", | ||
| type_schema: TypeSchema::String, |
There was a problem hiding this comment.
The controller schema uses type_schema, TypeSchema::Float, and TypeSchema::Boolean, but FieldSchema is defined with a ty field and the enum variants are F64/Bool. As soon as the syntax error in langfuse.rs is fixed, this new RPC module still prevents the Rust core from compiling.
Useful? React with 👍 / 👎.
| void callCoreRpc('openhuman.observability_submit_score', { | ||
| trace_id: `${selectedThreadId}:${msg.id}`, | ||
| name: 'user-feedback', | ||
| value: 1.0, | ||
| }); |
There was a problem hiding this comment.
callCoreRpc is declared as callCoreRpc({ method, params, ... }), so passing the method string and params as two arguments fails typechecking and would leave method undefined at runtime when the thumbs-up button is clicked. Use the existing object-shaped call here so the feedback RPC can actually be sent.
Useful? React with 👍 / 👎.
| className="rounded-full bg-white p-1 text-stone-500 shadow-sm ring-1 ring-inset ring-stone-200 hover:bg-stone-50 hover:text-stone-700 dark:bg-stone-800 dark:text-stone-400 dark:ring-stone-700 dark:hover:bg-stone-700 dark:hover:text-stone-200" | ||
| onClick={() => { | ||
| void callCoreRpc('openhuman.observability_submit_score', { | ||
| trace_id: `${selectedThreadId}:${msg.id}`, |
There was a problem hiding this comment.
The trace id produced by the progress bridge is trace_session_id(metadata.session_id, thread_id):request_id, where request_id is the UUID returned/emitted by the chat turn, not the persisted assistant message id. In normal chat this sends threadId:msg_..., so Langfuse receives a score for a trace that was never created and user feedback will not attach to the run unless the real request/trace id is persisted with the message and used here.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
app/src/pages/Conversations.tsx (1)
2214-2227: 📐 Maintainability & Code Quality | 🔵 TrivialDuplicate button logic could be extracted into a single helper.
Both thumbs buttons repeat the same
callCoreRpcshape, differing only invalue/icon/title. A smallsubmitFeedback(value: number)closure would reduce duplication.Also applies to: 2228-2241
🤖 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 `@app/src/pages/Conversations.tsx` around lines 2214 - 2227, Both thumbs buttons in Conversations.tsx duplicate the same observability_submit_score call shape, differing only by feedback value, icon, and title. Extract the shared logic into a small submitFeedback(value: number) helper/closure near the button handlers, and have the thumbs-up and thumbs-down buttons call it with 1.0 and -1.0 respectively while keeping their existing labels and icons.src/openhuman/agent/progress_tracing/langfuse.rs (1)
418-418: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant timestamp round-trip.
chrono::Utc::now().timestamp_millis() as u64is immediately cast back toi64insideiso_millis. Simplifying to a directchrono::Utc::now().to_rfc3339()(or passing thei64through without the intermediateu64cast) avoids the pointless conversion.♻️ Simplify timestamp construction
- "timestamp": iso_millis(chrono::Utc::now().timestamp_millis() as u64), + "timestamp": chrono::Utc::now().to_rfc3339(),🤖 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/langfuse.rs` at line 418, The timestamp construction in the langfuse progress tracing code is doing a redundant round-trip through u64 before being converted back inside iso_millis. Update the timestamp generation near the langfuse event payload to either pass the chrono::Utc::now().timestamp_millis() value directly into iso_millis without the cast, or switch the field to use chrono::Utc::now().to_rfc3339() if that better matches the expected format. Keep the change localized to the timestamp assembly in the progress tracing flow.
🤖 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 `@app/src/pages/Conversations.tsx`:
- Around line 2217-2223: The feedback action in Conversations.tsx is
fire-and-forget but `callCoreRpc('openhuman.observability_submit_score', ...)`
can reject with `CoreRpcError`, so the click can fail as an unhandled promise
rejection. Update the onClick handlers that submit the score to attach a
`.catch()` (or wrap in try/catch with an async handler) and handle the failure
from `callCoreRpc` in both the `user-feedback` and related score submission
sites. Make sure the failure path is explicitly handled and, if appropriate,
surface a local user-facing indication that the submission did not register.
- Line 2224: The tooltip titles in Conversations are hardcoded English strings
and should be routed through i18n. Update the `Conversations` component to use
the existing `useT()`/`t(...)` pattern for the `title` values currently set to
“Good response” and “Bad response”, matching the rest of the UI text in this
file. Make sure the tooltip labels use translation keys instead of literals, and
keep the change localized to the affected response feedback elements.
- Around line 2212-2243: The feedback buttons in Conversations.tsx are
submitting scores with the wrong trace key, so they won’t attach to the agent
turn trace. Update the `onClick` handlers near
`callCoreRpc('openhuman.observability_submit_score', ...)` to use the turn’s
Langfuse trace id format (`<session_or_thread_id>:<request_id>`) instead of
`${selectedThreadId}:${msg.id}`. Use the existing `selectedThreadId`, `msg`, and
the turn/request identifiers available in this render path to build the correct
`trace_id` before calling `openhuman.observability_submit_score`.
In `@src/openhuman/agent/progress_tracing/rpc.rs`:
- Around line 72-89: The `handle_submit_score` RPC handler currently discards
the `langfuse::push_score` result, so failures are invisible. Update
`handle_submit_score` in `rpc.rs` to inspect the awaited `push_score` result
and, on `Err`, emit a warning log with the error details (matching the
observability style used by `push_spans` in `progress_tracing.rs`) while still
returning the existing `SubmitScoreResponse { ok: true }`.
---
Nitpick comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 2214-2227: Both thumbs buttons in Conversations.tsx duplicate the
same observability_submit_score call shape, differing only by feedback value,
icon, and title. Extract the shared logic into a small submitFeedback(value:
number) helper/closure near the button handlers, and have the thumbs-up and
thumbs-down buttons call it with 1.0 and -1.0 respectively while keeping their
existing labels and icons.
In `@src/openhuman/agent/progress_tracing/langfuse.rs`:
- Line 418: The timestamp construction in the langfuse progress tracing code is
doing a redundant round-trip through u64 before being converted back inside
iso_millis. Update the timestamp generation near the langfuse event payload to
either pass the chrono::Utc::now().timestamp_millis() value directly into
iso_millis without the cast, or switch the field to use
chrono::Utc::now().to_rfc3339() if that better matches the expected format. Keep
the change localized to the timestamp assembly in the progress tracing flow.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9931d67d-a61f-4920-bc17-f46a574b6b62
📒 Files selected for processing (5)
app/src/pages/Conversations.tsxsrc/core/all.rssrc/openhuman/agent/progress_tracing.rssrc/openhuman/agent/progress_tracing/langfuse.rssrc/openhuman/agent/progress_tracing/rpc.rs
coderabbitai
Bot
added
the
agent
salarkhannn
force-pushed
the
feature/issue-4496
branch
from
d9f1287 to
a2787d9
Compare
coderabbitai
Bot
removed
rust-core
coderabbitai
Bot
added
agent
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/pages/__tests__/Conversations.feedback.test.tsx (2)
218-226: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for the "only latest agent message" gating.
Per the referenced implementation (
app/src/pages/Conversations.tsx:2197-2281), feedback buttons only render whenlatestVisibleMessage?.id === msg.id. No test in this suite exercises a thread with multiple agent messages to confirm older agent messages don't get feedback buttons while the latest one does. This is core behavior the RPC-shape tests don't cover.Also applies to: 311-348
🤖 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 `@app/src/pages/__tests__/Conversations.feedback.test.tsx` around lines 218 - 226, Add test coverage for the latest-agent-message feedback gating in Conversations.feedback.test.tsx by rendering a thread with multiple agent messages and asserting that only the message matching latestVisibleMessage in Conversations.tsx gets the Good response/Bad response buttons. Update the relevant tests around renderWithFeedback and the thumbs-up/thumbs-down assertions so older agent messages are verified to have no feedback controls while the newest visible agent message still does.
150-194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce duplication by parameterizing
renderWithFeedback.The store/thread/message setup and render boilerplate in
renderWithFeedback(lines 150-194) is copy-pasted almost verbatim in the "custom traceId" test (lines 256-298) and the "no agent messages" test (lines 311-343). Consider extendingrenderWithFeedbackto acceptmessages/thread overrides so all three scenarios reuse a single helper.♻️ Suggested refactor
-async function renderWithFeedback() { - const thread = makeThread({ id: 'feedback-thread', title: 'Feedback Thread' }); - const messages: ThreadMessage[] = [ - { - id: 'backend-trace-123', - sender: 'agent', - type: 'text', - content: 'Here is my response to your question.', - extraMetadata: {}, - createdAt: '2026-01-01T00:01:00.000Z', - }, - ]; +async function renderWithFeedback( + overrides: { thread?: Partial<Thread>; messages?: ThreadMessage[] } = {} +) { + const thread = makeThread({ id: 'feedback-thread', title: 'Feedback Thread', ...overrides.thread }); + const messages: ThreadMessage[] = overrides.messages ?? [ + { + id: 'backend-trace-123', + sender: 'agent', + type: 'text', + content: 'Here is my response to your question.', + extraMetadata: {}, + createdAt: '2026-01-01T00:01:00.000Z', + }, + ];Then the two duplicated blocks can call
renderWithFeedback({ thread: { id: 'trace-id-thread' }, messages: [...] })instead of re-implementing the store/render wiring.Also applies to: 256-298, 311-343
🤖 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 `@app/src/pages/__tests__/Conversations.feedback.test.tsx` around lines 150 - 194, The test setup in renderWithFeedback is duplicated across the feedback scenarios; refactor it into a single parameterized helper so the "custom traceId" and "no agent messages" tests reuse the same store/render wiring. Update renderWithFeedback to accept thread and messages overrides (and any needed defaults) while keeping the existing rendering flow with Conversations, Provider, MemoryRouter, SidebarSlotProvider, and SidebarSlotOutlet. Then replace the repeated setup blocks in the other tests with calls to this helper using the appropriate thread/message inputs.
🤖 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 `@app/src/pages/__tests__/Conversations.feedback.test.tsx`:
- Around line 218-226: Add test coverage for the latest-agent-message feedback
gating in Conversations.feedback.test.tsx by rendering a thread with multiple
agent messages and asserting that only the message matching latestVisibleMessage
in Conversations.tsx gets the Good response/Bad response buttons. Update the
relevant tests around renderWithFeedback and the thumbs-up/thumbs-down
assertions so older agent messages are verified to have no feedback controls
while the newest visible agent message still does.
- Around line 150-194: The test setup in renderWithFeedback is duplicated across
the feedback scenarios; refactor it into a single parameterized helper so the
"custom traceId" and "no agent messages" tests reuse the same store/render
wiring. Update renderWithFeedback to accept thread and messages overrides (and
any needed defaults) while keeping the existing rendering flow with
Conversations, Provider, MemoryRouter, SidebarSlotProvider, and
SidebarSlotOutlet. Then replace the repeated setup blocks in the other tests
with calls to this helper using the appropriate thread/message inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16018f30-9f67-42d3-8e1d-d98cdc717e2a
📒 Files selected for processing (5)
GEMINI.mdapp/src/pages/__tests__/Conversations.feedback.test.tsxsrc/openhuman/agent/progress_tracing/rpc.rssrc/openhuman/threads/tools.rsvendor/tinyagents
🚧 Files skipped from review as they are similar to previous changes (2)
- src/openhuman/threads/tools.rs
- src/openhuman/agent/progress_tracing/rpc.rs
|
Just pushed the core implementation for the Langfuse scores, along with the unit tests to pass the coverage gates. |
salarkhannn
force-pushed
the
feature/issue-4496
branch
from
1d9fa3d to
6989882
Compare
Fixes #4496.
This adds the
push_scoremethod to the Langfuse emitter and exposes it over RPC asopenhuman.observability_submit_score. It also updates the assistant message UI to include positive and negative feedback buttons which submit quality scores to the active trace.Summary by CodeRabbit