feat: thumbs up/down on ai assistant messages#18803
Open
ErlingHauan wants to merge 26 commits into
Open
Conversation
github-actions
Bot
added
quality/testing
Contributor
|
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 end-user thumbs-up/down feedback: Langfuse helpers and FastAPI feedback endpoint, C# Altinity client and controller, frontend feedback UI/types/hooks, trace-id mapping for persisted messages, API paths/mutations, and tests across services. ChangesBackend feedback capture and API
Frontend feedback UI and state management
Sequence DiagramsequenceDiagram
participant UserClient
participant Frontend (AiAssistant)
participant StudioAssistant Components
participant Designer API (PUT /api/feedback/{traceId})
participant Langfuse
UserClient->>Frontend (AiAssistant): click thumbs-up/down + optional comment (traceId)
Frontend (AiAssistant)->>StudioAssistant Components: onMessageFeedback({traceId, payload})
StudioAssistant Components->>Designer API (PUT /api/feedback/{traceId}): send feedback with X-Developer header
Designer API (PUT /api/feedback/{traceId})->>Langfuse: score_validation(name="user_feedback", passed, trace_id, comment, score_id)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
src/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/Messages.module.css (1)
235-367: 💤 Low valueConsider updating remaining content selectors for consistency.
The heading selectors (h1, h2, h3) were correctly moved to
.assistantContentsince that's where the HTML content is injected. For consistency, consider updating the other content-related selectors (strong, em, code, pre, ul, li, p, ol, hr) to also use.assistantContentinstead of.assistantMessage. This would make the styling more explicit and maintainable, though the current selectors work correctly as descendant selectors.🤖 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/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/Messages.module.css` around lines 235 - 367, The CSS currently targets content elements using the .assistantMessage descendant selectors (e.g., .assistantMessage strong, em, code, pre, pre[data-language]::before, ul, li, li::marker, p, ol, hr, .assistantMessage *) but the actual injected HTML lives under .assistantContent; update all those selectors to use .assistantContent instead of .assistantMessage so styles apply explicitly to the rendered message content (keep the selector structure and specific rules intact, only replace the namespace marker .assistantMessage → .assistantContent for the listed rules).src/AI/agents/api/routes/feedback.py (1)
58-64: 💤 Low value
score_validationfailure is invisible to the caller.
score_validationcatches all internal exceptions and only logs atdebuglevel, so the endpoint will return204even when the Langfuse write actually failed. This is probably the intent ("best-effort feedback"), but it does mean retries, monitoring, and the frontend's success UI will all report success on a silent failure. Consider either:
- Logging at
warning/infofrom this endpoint whenscore_validationdid not record a score (would require a return value fromscore_validation), or- Emitting a structured log line here on entry/exit (
trace_id,thumbs_up,caller) so feedback submissions are auditable even when Langfuse is degraded.🤖 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/AI/agents/api/routes/feedback.py` around lines 58 - 64, The endpoint currently calls score_validation(FEEDBACK_SCORE_NAME, passed=req.thumbs_up, trace_id=req.trace_id, comment=req.comment) and always returns Response(status_code=204) even if score_validation swallowed errors; change this so failures are visible: either make score_validation return a boolean/enum indicating success/failure and check its return here to log a warning (including trace_id, req.thumbs_up, and caller context) before returning 204, or wrap the call in a try/except that logs a structured warning containing trace_id, thumbs_up, FEEDBACK_SCORE_NAME and the caught exception when the write fails; ensure the log level is warning/info rather than debug so failures are auditable.src/AI/agents/shared/utils/langfuse_utils.py (1)
266-278: 💤 Low valueAdd a short docstring to
get_current_trace_id.The surrounding helpers (
get_trace_developer,_has_active_trace,flush_langfuse, etc.) all carry docstrings. A one-liner onget_current_trace_idkeeps the module style consistent and clarifies the "returns None when Langfuse is disabled or any error occurs" contract that_has_active_tracenow depends on.🤖 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/AI/agents/shared/utils/langfuse_utils.py` around lines 266 - 278, Add a one-line docstring to get_current_trace_id explaining that it returns the current Langfuse trace ID or None when Langfuse is disabled or an error occurs; reference the function name get_current_trace_id and note its interaction with is_langfuse_enabled, get_client().get_current_trace_id(), and the dependent helper _has_active_trace so the contract is explicit and consistent with surrounding helpers.src/Designer/backend/src/Designer/Models/Dto/ChatFeedbackRequest.cs (1)
5-9: 💤 Low valueConsider adding explicit JsonPropertyName attributes.
Whilst ASP.NET Core's default deserialisation handles casing, DTOs in this project typically use explicit
[JsonPropertyName]attributes to document the JSON contract clearly. As per coding guidelines, this improves maintainability and avoids ambiguity when the frontend evolves.📝 Proposed improvement
+using System.Text.Json.Serialization; + namespace Altinn.Studio.Designer.Models.Dto; public record ChatFeedbackRequest( - [MinLength(1), MaxLength(64)] string TraceId, + [JsonPropertyName("traceId"), MinLength(1), MaxLength(64)] string TraceId, + [JsonPropertyName("thumbsUp")] bool ThumbsUp, - [MaxLength(10000)] string? Comment + [JsonPropertyName("comment"), MaxLength(10000)] string? Comment );🤖 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/Designer/backend/src/Designer/Models/Dto/ChatFeedbackRequest.cs` around lines 5 - 9, Update the ChatFeedbackRequest record to explicitly annotate each positional property with JsonPropertyName attributes to document the JSON contract: add [JsonPropertyName("traceId")] for TraceId, [JsonPropertyName("thumbsUp")] for ThumbsUp and [JsonPropertyName("comment")] for Comment on the ChatFeedbackRequest record declaration so the DTO matches the project's explicit JSON naming convention and avoids casing ambiguity.src/Designer/backend/src/Designer/Services/Implementation/Altinity/AltinityAgentClient.cs (2)
27-27: 💤 Low valueUse UriBuilder for more robust URI construction.
String concatenation of URLs can produce malformed URIs if
AgentUrlhas a trailing slash. UsingUriBuilderor ensuring consistent slash handling prevents potential issues.🔧 Proposed improvement
- var requestUri = new Uri($"{_altinitySettings.AgentUrl}{FeedbackPath}"); + var baseUri = new Uri(_altinitySettings.AgentUrl.TrimEnd('/')); + var requestUri = new Uri(baseUri, FeedbackPath.TrimStart('/'));🤖 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/Designer/backend/src/Designer/Services/Implementation/Altinity/AltinityAgentClient.cs` at line 27, The code builds requestUri via string concatenation which can produce malformed URIs when _altinitySettings.AgentUrl has or lacks a trailing slash; update the AltinityAgentClient method that creates requestUri to use UriBuilder (or normalize/truncate trailing slash on _altinitySettings.AgentUrl) and append FeedbackPath safely so the resulting Uri is well-formed—replace the line constructing requestUri with logic that combines _altinitySettings.AgentUrl and FeedbackPath using UriBuilder (or explicit slash handling) to ensure correct Uri creation.
25-27: ⚡ Quick winAdd CancellationToken parameter for proper async cancellation.
The
SendFeedbackAsyncmethod does not accept aCancellationToken, which prevents callers from cancelling long-running HTTP requests. This is particularly important for HTTP operations that may time out or when the user navigates away.♻️ Proposed improvement
- public async Task SendFeedbackAsync(string developer, string traceId, bool thumbsUp, string? comment) + public async Task SendFeedbackAsync(string developer, string traceId, bool thumbsUp, string? comment, CancellationToken cancellationToken = default) { var requestUri = new Uri($"{_altinitySettings.AgentUrl}{FeedbackPath}"); using var httpRequest = new HttpRequestMessage(HttpMethod.Post, requestUri) { Content = JsonContent.Create( new { trace_id = traceId, thumbs_up = thumbsUp, comment, } ), }; httpRequest.Headers.Add(DeveloperHeader, developer); - using var response = await _httpClient.SendAsync(httpRequest); + using var response = await _httpClient.SendAsync(httpRequest, cancellationToken); if (!response.IsSuccessStatusCode) { string responseContent = await response.Content.ReadAsStringAsync(); throw new HttpRequestException($"Altinity feedback returned {response.StatusCode}: {responseContent}"); } }Update the interface signature accordingly:
- Task SendFeedbackAsync(string developer, string traceId, bool thumbsUp, string? comment); + Task SendFeedbackAsync(string developer, string traceId, bool thumbsUp, string? comment, CancellationToken cancellationToken = default);🤖 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/Designer/backend/src/Designer/Services/Implementation/Altinity/AltinityAgentClient.cs` around lines 25 - 27, The SendFeedbackAsync method lacks a CancellationToken parameter; update the method signature for SendFeedbackAsync(string developer, string traceId, bool thumbsUp, string? comment, CancellationToken cancellationToken) and make the corresponding change in its interface, then pass that token into any HTTP calls inside (e.g., HttpClient.PostAsync/SendAsync) and to any internal async operations so the HTTP request to _altinitySettings.AgentUrl + FeedbackPath can be cancelled; also update all callers to accept/forward a CancellationToken.src/Designer/backend/src/Designer/Controllers/ChatController.cs (1)
138-145: 💤 Low valueClarify the purpose of unused org and app route parameters.
The
organdapproute parameters are not used in the method body, unlike other endpoints in this controller which construct anAltinnRepoEditingContext. Whilst this may be intentional for route consistency, consider either:
- Documenting why they're not needed (e.g., validation happens downstream in the Python service)
- Using them to add an authorization check
- Simplifying the route if they're truly unnecessary
🤖 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/Designer/backend/src/Designer/Controllers/ChatController.cs` around lines 138 - 145, The route parameters org and app are unused in SubmitFeedback (unlike other endpoints that build an AltinnRepoEditingContext) — either remove them from the route signature or actually use them to authorise/validate the request before forwarding to altinityAgentClient.SendFeedbackAsync; specifically, update SubmitFeedback to build an AltinnRepoEditingContext (or call your existing authorization helper) from org and app and enforce/validate authorization or repository context, then call altinityAgentClient.SendFeedbackAsync(developer, request.TraceId, request.ThumbsUp, request.Comment); if the omission is intentional, add a brief comment above SubmitFeedback explaining why org/app are not needed (e.g., downstream validation in Python) to make intent explicit.
🤖 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/AI/agents/api/routes/feedback.py`:
- Around line 19-42: The trace_id validator in FeedbackReq (_validate_trace_id)
rejects whitespace-only strings but returns the original value, so update
_validate_trace_id to strip surrounding whitespace (use v = v.strip()), validate
non-emptiness on the stripped value, and return the trimmed trace_id so
downstream calls like get_trace_developer and score_validation receive a
normalized, whitespace-trimmed identifier.
In `@src/AI/agents/shared/utils/langfuse_utils.py`:
- Around line 240-263: Add a brief docstring to the get_current_trace_id
function describing what it returns and its behavior (e.g., "Return the current
Langfuse trace id from the client if available, otherwise None"), matching the
style used in neighboring functions like get_trace_developer; locate the
get_current_trace_id function and insert a one-line docstring immediately below
its def line that explains the return value and when it may be None.
In
`@src/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/MessageFeedback/MessageFeedback.tsx`:
- Around line 18-19: The submit handler currently calls onSubmit and immediately
clears the dialog state, which can drop user feedback on failure; change the
handler in MessageFeedback to treat onSubmit as async (await its Promise), add
an isSubmitting flag to guard against duplicate submissions, set
isSubmitting=true before calling await onSubmit(feedback), and only close/reset
the dialog state (clear feedback text and close dialog) after onSubmit resolves
successfully; in catch/finally re-enable isSubmitting and keep the dialog open
so the user can retry (optionally surface an error message).
In
`@src/Designer/frontend/packages/shared/src/hooks/mutations/useChatFeedbackMutation.test.ts`:
- Around line 17-18: The test is calling renderHookWithProviders incorrectly by
invoking it twice; update the call site to pass the hook directly: replace the
nested call pattern using renderHookWithProviders()(() =>
useChatFeedbackMutation()).renderHookResult.result with a single call
renderHookWithProviders(() => useChatFeedbackMutation()).result so the hook
function is the first argument and you consume .result directly from the
returned RenderHookResult (referencing renderHookWithProviders and
useChatFeedbackMutation to locate the change).
---
Nitpick comments:
In `@src/AI/agents/api/routes/feedback.py`:
- Around line 58-64: The endpoint currently calls
score_validation(FEEDBACK_SCORE_NAME, passed=req.thumbs_up,
trace_id=req.trace_id, comment=req.comment) and always returns
Response(status_code=204) even if score_validation swallowed errors; change this
so failures are visible: either make score_validation return a boolean/enum
indicating success/failure and check its return here to log a warning (including
trace_id, req.thumbs_up, and caller context) before returning 204, or wrap the
call in a try/except that logs a structured warning containing trace_id,
thumbs_up, FEEDBACK_SCORE_NAME and the caught exception when the write fails;
ensure the log level is warning/info rather than debug so failures are
auditable.
In `@src/AI/agents/shared/utils/langfuse_utils.py`:
- Around line 266-278: Add a one-line docstring to get_current_trace_id
explaining that it returns the current Langfuse trace ID or None when Langfuse
is disabled or an error occurs; reference the function name get_current_trace_id
and note its interaction with is_langfuse_enabled,
get_client().get_current_trace_id(), and the dependent helper _has_active_trace
so the contract is explicit and consistent with surrounding helpers.
In `@src/Designer/backend/src/Designer/Controllers/ChatController.cs`:
- Around line 138-145: The route parameters org and app are unused in
SubmitFeedback (unlike other endpoints that build an AltinnRepoEditingContext) —
either remove them from the route signature or actually use them to
authorise/validate the request before forwarding to
altinityAgentClient.SendFeedbackAsync; specifically, update SubmitFeedback to
build an AltinnRepoEditingContext (or call your existing authorization helper)
from org and app and enforce/validate authorization or repository context, then
call altinityAgentClient.SendFeedbackAsync(developer, request.TraceId,
request.ThumbsUp, request.Comment); if the omission is intentional, add a brief
comment above SubmitFeedback explaining why org/app are not needed (e.g.,
downstream validation in Python) to make intent explicit.
In `@src/Designer/backend/src/Designer/Models/Dto/ChatFeedbackRequest.cs`:
- Around line 5-9: Update the ChatFeedbackRequest record to explicitly annotate
each positional property with JsonPropertyName attributes to document the JSON
contract: add [JsonPropertyName("traceId")] for TraceId,
[JsonPropertyName("thumbsUp")] for ThumbsUp and [JsonPropertyName("comment")]
for Comment on the ChatFeedbackRequest record declaration so the DTO matches the
project's explicit JSON naming convention and avoids casing ambiguity.
In
`@src/Designer/backend/src/Designer/Services/Implementation/Altinity/AltinityAgentClient.cs`:
- Line 27: The code builds requestUri via string concatenation which can produce
malformed URIs when _altinitySettings.AgentUrl has or lacks a trailing slash;
update the AltinityAgentClient method that creates requestUri to use UriBuilder
(or normalize/truncate trailing slash on _altinitySettings.AgentUrl) and append
FeedbackPath safely so the resulting Uri is well-formed—replace the line
constructing requestUri with logic that combines _altinitySettings.AgentUrl and
FeedbackPath using UriBuilder (or explicit slash handling) to ensure correct Uri
creation.
- Around line 25-27: The SendFeedbackAsync method lacks a CancellationToken
parameter; update the method signature for SendFeedbackAsync(string developer,
string traceId, bool thumbsUp, string? comment, CancellationToken
cancellationToken) and make the corresponding change in its interface, then pass
that token into any HTTP calls inside (e.g., HttpClient.PostAsync/SendAsync) and
to any internal async operations so the HTTP request to
_altinitySettings.AgentUrl + FeedbackPath can be cancelled; also update all
callers to accept/forward a CancellationToken.
In
`@src/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/Messages.module.css`:
- Around line 235-367: The CSS currently targets content elements using the
.assistantMessage descendant selectors (e.g., .assistantMessage strong, em,
code, pre, pre[data-language]::before, ul, li, li::marker, p, ol, hr,
.assistantMessage *) but the actual injected HTML lives under .assistantContent;
update all those selectors to use .assistantContent instead of .assistantMessage
so styles apply explicitly to the rendered message content (keep the selector
structure and specific rules intact, only replace the namespace marker
.assistantMessage → .assistantContent for the listed rules).
🪄 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
Run ID: 46b8bf43-846d-4522-aeb4-e05c759f7158
📒 Files selected for processing (45)
src/AI/agents/agents/graph/nodes/assistant_node.pysrc/AI/agents/agents/graph/nodes/reviewer_node.pysrc/AI/agents/api/main.pysrc/AI/agents/api/routes/__init__.pysrc/AI/agents/api/routes/feedback.pysrc/AI/agents/shared/utils/langfuse_utils.pysrc/AI/agents/tests/api/test_feedback.pysrc/Designer/backend/src/Designer/Controllers/ChatController.cssrc/Designer/backend/src/Designer/Models/Dto/ChatFeedbackRequest.cssrc/Designer/backend/src/Designer/Program.cssrc/Designer/backend/src/Designer/Services/Implementation/Altinity/AltinityAgentClient.cssrc/Designer/backend/src/Designer/Services/Interfaces/Altinity/IAltinityAgentClient.cssrc/Designer/backend/tests/Designer.Tests/Controllers/ChatController/SubmitFeedbackTests.cssrc/Designer/frontend/AGENTS.mdsrc/Designer/frontend/app-development/features/aiAssistant/AiAssistant.tsxsrc/Designer/frontend/app-development/features/aiAssistant/hooks/useAltinityAssistant/useAltinityAssistant.test.tssrc/Designer/frontend/app-development/features/aiAssistant/hooks/useAltinityAssistant/useAltinityAssistant.tssrc/Designer/frontend/app-development/features/aiAssistant/hooks/useAltinityThreads/useAltinityThreads.test.tssrc/Designer/frontend/app-development/features/aiAssistant/hooks/useAltinityThreads/useAltinityThreads.tssrc/Designer/frontend/app-development/features/aiAssistant/hooks/useAltinityWorkflow/useAltinityWorkflow.tssrc/Designer/frontend/app-development/features/aiAssistant/utils/messageUtils.test.tssrc/Designer/frontend/app-development/features/aiAssistant/utils/messageUtils.tssrc/Designer/frontend/language/src/nb.jsonsrc/Designer/frontend/libs/studio-assistant/src/Assistant/Assistant.tsxsrc/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/ChatColumn.tsxsrc/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/MessageFeedback/MessageFeedback.module.csssrc/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/MessageFeedback/MessageFeedback.test.tsxsrc/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/MessageFeedback/MessageFeedback.tsxsrc/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/MessageFeedback/index.tssrc/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/Messages.module.csssrc/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/Messages.tsxsrc/Designer/frontend/libs/studio-assistant/src/components/CompleteInterface/CompleteInterface.tsxsrc/Designer/frontend/libs/studio-assistant/src/index.tssrc/Designer/frontend/libs/studio-assistant/src/mocks/mockTexts.tssrc/Designer/frontend/libs/studio-assistant/src/types/AssistantConfig.tssrc/Designer/frontend/libs/studio-assistant/src/types/AssistantTexts.tssrc/Designer/frontend/libs/studio-assistant/src/types/ChatThread.tssrc/Designer/frontend/libs/studio-assistant/src/types/UserFeedback.tssrc/Designer/frontend/packages/shared/src/api/mutations.tssrc/Designer/frontend/packages/shared/src/api/paths.jssrc/Designer/frontend/packages/shared/src/hooks/mutations/useChatFeedbackMutation.test.tssrc/Designer/frontend/packages/shared/src/hooks/mutations/useChatFeedbackMutation.tssrc/Designer/frontend/packages/shared/src/mocks/queriesMock.tssrc/Designer/frontend/packages/shared/src/types/api/ChatFeedbackPayload.tssrc/Designer/frontend/packages/shared/src/types/api/index.ts
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #18803 +/- ##
==========================================
+ Coverage 95.71% 95.83% +0.12%
==========================================
Files 2587 3021 +434
Lines 33007 39625 +6618
Branches 4080 4858 +778
==========================================
+ Hits 31593 37976 +6383
- Misses 1069 1233 +164
- Partials 345 416 +71 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Ildest
approved these changes
May 18, 2026
Contributor
Ildest
left a comment
Ildest
left a comment
There was a problem hiding this comment.
Hei @ErlingHauan :-). Har lagt inn noen forslag, du kan se om du liker dem.
Contributor
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 `@src/Designer/backend/src/Designer/Controllers/ChatController.cs`:
- Around line 138-145: SubmitFeedback action is missing a CancellationToken
parameter; update the method signature of SubmitFeedback to accept a
CancellationToken (e.g., CancellationToken cancellationToken) and pass that
token into altinityAgentClient.SendFeedbackAsync so the request can be
cancelled; also update IAltinityAgentClient.SendFeedbackAsync signature and all
concrete implementations to accept the CancellationToken and forward it through
any downstream async calls (use HttpContext.RequestAborted when wiring callers
if needed), and ensure the AuthenticationHelper.GetDeveloperUserName usage stays
the same.
In `@src/Designer/backend/src/Designer/Models/Dto/ChatFeedbackRequest.cs`:
- Line 5: Extract the hard-coded 10000 into a named constant and use that
constant in the MaxLength attribute; for example add a public const int
MaxCommentLength = 10000 (either as a member of the ChatFeedbackRequest record
or in a shared ValidationConstants/ChatConstants class) and replace
[MaxLength(10000)] with [MaxLength(MaxCommentLength)] referencing that constant
so the MaxLength attribute uses the named symbol.
- Line 5: The ChatFeedbackRequest DTO currently serializes the ThumbsUp property
with default casing; update the ChatFeedbackRequest record to add a
JsonPropertyName attribute on the ThumbsUp property to serialize as "thumbs_up"
(use a string literal per guidelines) and likewise ensure any other properties
that must match the Python API (e.g., Comment if needed) have JsonPropertyName
attributes; reference the ChatFeedbackRequest type and the ThumbsUp property
when making this change so the JSON contract matches feedback.py.
In
`@src/Designer/backend/src/Designer/Services/Implementation/Altinity/AltinityAgentClient.cs`:
- Around line 25-40: Add a CancellationToken parameter to SendFeedbackAsync and
thread it through to the HTTP call: update the method signature of
SendFeedbackAsync(string developer, string traceId, bool thumbsUp, string?
comment, CancellationToken cancellationToken = default), pass cancellationToken
into _httpClient.SendAsync(httpRequest, cancellationToken), and update the
IAltinityAgentClient interface signature and the call site in
ChatController.SubmitFeedback to accept and forward the token (or supply
CancellationToken.None where appropriate).
🪄 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
Run ID: a2856a5b-fe75-46e6-ac78-387cc2fab615
📒 Files selected for processing (16)
src/AI/agents/api/routes/feedback.pysrc/AI/agents/shared/utils/langfuse_utils.pysrc/AI/agents/tests/api/test_feedback.pysrc/Designer/backend/src/Designer/Controllers/ChatController.cssrc/Designer/backend/src/Designer/Models/Dto/ChatFeedbackRequest.cssrc/Designer/backend/src/Designer/Services/Implementation/Altinity/AltinityAgentClient.cssrc/Designer/backend/tests/Designer.Tests/Controllers/ChatController/SubmitFeedbackTests.cssrc/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/MessageFeedback/MessageFeedback.test.tsxsrc/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/MessageFeedback/MessageFeedback.tsxsrc/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/Messages.tsxsrc/Designer/frontend/libs/studio-assistant/src/types/UserFeedback.tssrc/Designer/frontend/packages/shared/src/api/mutations.tssrc/Designer/frontend/packages/shared/src/api/paths.jssrc/Designer/frontend/packages/shared/src/hooks/mutations/useChatFeedbackMutation.test.tssrc/Designer/frontend/packages/shared/src/hooks/mutations/useChatFeedbackMutation.tssrc/Designer/frontend/packages/shared/src/types/api/ChatFeedbackPayload.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Designer/backend/tests/Designer.Tests/Controllers/ChatController/SubmitFeedbackTests.cs
- src/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/MessageFeedback/MessageFeedback.test.tsx
- src/Designer/frontend/packages/shared/src/hooks/mutations/useChatFeedbackMutation.ts
- src/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/Messages.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/Designer/backend/src/Designer/Services/Implementation/Altinity/AltinityAgentClient.cs`:
- Line 34: Summary: The code builds requestUri by concatenating traceId directly
into the path, which can break URIs when traceId contains reserved characters.
Fix: URL-encode the traceId path segment before appending to the base AgentUrl
(use Uri.EscapeDataString(traceId) or equivalent) and construct the final Uri
using UriBuilder or new Uri(baseUri, relativePath) to avoid double slashes;
update the code that sets requestUri in AltinityAgentClient (referencing
_altinitySettings.AgentUrl, FeedbackPathPrefix, and traceId) so the path segment
is encoded and the Uri is constructed safely.
🪄 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
Run ID: 30e641f3-25ea-47d3-ac4e-d10251a1c00b
📒 Files selected for processing (9)
src/Designer/backend/src/Designer/Controllers/ChatController.cssrc/Designer/backend/src/Designer/Services/Implementation/Altinity/AltinityAgentClient.cssrc/Designer/backend/src/Designer/Services/Interfaces/Altinity/IAltinityAgentClient.cssrc/Designer/backend/tests/Designer.Tests/Controllers/ChatController/SubmitFeedbackTests.cssrc/Designer/frontend/app-development/features/aiAssistant/AiAssistant.tsxsrc/Designer/frontend/language/src/nb.jsonsrc/Designer/frontend/libs/studio-assistant/src/components/ChatColumn/Messages/MessageFeedback/MessageFeedback.tsxsrc/Designer/frontend/libs/studio-assistant/src/mocks/mockTexts.tssrc/Designer/frontend/libs/studio-assistant/src/types/AssistantTexts.ts
✅ Files skipped from review due to trivial changes (1)
- src/Designer/frontend/language/src/nb.json
🚧 Files skipped from review as they are similar to previous changes (5)
- src/Designer/backend/src/Designer/Services/Interfaces/Altinity/IAltinityAgentClient.cs
- src/Designer/backend/src/Designer/Controllers/ChatController.cs
- src/Designer/frontend/libs/studio-assistant/src/mocks/mockTexts.ts
- src/Designer/frontend/app-development/features/aiAssistant/AiAssistant.tsx
- src/Designer/backend/tests/Designer.Tests/Controllers/ChatController/SubmitFeedbackTests.cs
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
2 participants
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.
Description
Adds thumbs up/down buttons that a user can press to give feedback on the assistant's response. When pressed, it opens a dialog where the user can also type an optional message.
Solves digdir/digdir-ai-lab#144
Flow
Verification
Summary by CodeRabbit
New Features
Bug Fixes / Reliability
Tests