Repository navigation
Show Assistant Message Sources and Web Searches in Chat UI - #336
Conversation
📝 WalkthroughWalkthroughThis PR captures per-assistant-message metadata (web searches and source URLs): it adds a metadata column and model fields, accepts metadata in DAO inserts (Postgres/SQLite), collects metadata during agentic tool execution, serializes it for persistence, exposes it in protobuf responses, and renders it in the frontend message UI. ChangesEnd-to-end metadata support: collection, persistence, and display
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
…to sources-list
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for saving and displaying assistant message metadata (such as web search queries and source URLs) across the backend, database migrations, protobuf definitions, and frontend UI. Feedback on the changes highlights a critical bug in agentic_chat.go where a type assertion to []map[string]string on unmarshaled JSON will always fail, preventing search sources from being extracted. Additionally, there is duplicate code for the assistantMessageMetadata struct in both models.go and agentic_chat.go that should be consolidated by exporting the struct from the dao package.
I am having trouble creating individual review comments. Click here to see my feedback.
backend/chatservice/service/agentic_chat.go (391-399)
The type assertion resultMap["results"].([]map[string]string) will always fail when e.Result is unmarshaled from JSON. In Go, standard JSON unmarshaling into an any or map[string]any decodes JSON arrays as []any (i.e., []interface{}), and JSON objects as map[string]any. Attempting to type-assert directly to []map[string]string will result in ok being false, meaning no web search sources will ever be extracted and displayed in the UI.\n\nTo fix this, type-assert "results" to []any, iterate over the slice, type-assert each element to map[string]any, and then extract the "url" field as a string.
if resultMap, ok := e.Result.(map[string]any); ok {
if results, ok := resultMap["results"].([]any); ok {
for _, res := range results {
if resMap, ok := res.(map[string]any); ok {
if url, ok := resMap["url"].(string); ok {
if trimmed := strings.TrimSpace(url); trimmed != "" {
sourceURLs = append(sourceURLs, trimmed)
}
}
}
}
}
}backend/chatservice/dao/models.go (110-157)
The assistantMessageMetadata struct and its sub-structs are duplicated exactly between backend/chatservice/dao/models.go and backend/chatservice/service/agentic_chat.go. Since the service package already imports the dao package, we can eliminate this duplication by exporting these structs from the dao package (e.g., renaming them to AssistantMessageMetadataJSON, AssistantMetadataWebSearch, and AssistantMetadataSource) and reusing them in the service package.
type AssistantMessageMetadataJSON struct {
WebSearches []AssistantMetadataWebSearch `json:"websearches,omitempty"`
Sources []AssistantMetadataSource `json:"sources,omitempty"`
}
type AssistantMetadataWebSearch struct {
Query string `json:"query"`
}
type AssistantMetadataSource struct {
URL string `json:"url"`
}
func ParseAssistantMessageMetadataJSON(raw string) *proto.AssistantMessageMetadata {
if strings.TrimSpace(raw) == "" {
return nil
}
var metadata AssistantMessageMetadataJSON
if err := json.Unmarshal([]byte(raw), &metadata); err != nil {
slog.Error("dao:ParseAssistantMessageMetadataJSON", "message", "failed to unmarshal assistant metadata", "error", err)
return nil
}
protoMetadata := &proto.AssistantMessageMetadata{}
for _, search := range metadata.WebSearches {
if strings.TrimSpace(search.Query) == "" {
continue
}
protoMetadata.Websearches = append(protoMetadata.Websearches, &proto.AssistantMessageMetadata_WebSearch{
Query: search.Query,
})
}
for _, source := range metadata.Sources {
if strings.TrimSpace(source.URL) == "" {
continue
}
protoMetadata.Sources = append(protoMetadata.Sources, &proto.AssistantMessageMetadata_Source{
Url: source.URL,
})
}
if len(protoMetadata.Websearches) == 0 && len(protoMetadata.Sources) == 0 {
return nil
}
return protoMetadata
}backend/chatservice/service/agentic_chat.go (19-84)
To eliminate code duplication, we can remove the local definitions of assistantMessageMetadata and its sub-structs, and instead reuse the exported dao.AssistantMessageMetadataJSON and related structs from the dao package.
func buildAssistantMessageMetadata(webSearchQueries []string, sourceURLs []string) (*pb.AssistantMessageMetadata, string) {
metadata := dao.AssistantMessageMetadataJSON{}
seenQueries := make(map[string]struct{})
seenSources := make(map[string]struct{})
for _, query := range webSearchQueries {
trimmed := strings.TrimSpace(query)
if trimmed == "" {
continue
}
if _, exists := seenQueries[trimmed]; exists {
continue
}
seenQueries[trimmed] = struct{}{}
metadata.WebSearches = append(metadata.WebSearches, dao.AssistantMetadataWebSearch{Query: trimmed})
}
for _, sourceURL := range sourceURLs {
trimmed := strings.TrimSpace(sourceURL)
if trimmed == "" {
continue
}
if _, exists := seenSources[trimmed]; exists {
continue
}
seenSources[trimmed] = struct{}{}
metadata.Sources = append(metadata.Sources, dao.AssistantMetadataSource{URL: trimmed})
}
if len(metadata.WebSearches) == 0 && len(metadata.Sources) == 0 {
return nil, ""
}
payload, err := json.Marshal(metadata)
if err != nil {
slog.Error("service:Chat", "message", "failed to marshal assistant metadata", "error", err)
return nil, ""
}
protoMetadata := &pb.AssistantMessageMetadata{}
for _, search := range metadata.WebSearches {
protoMetadata.Websearches = append(protoMetadata.Websearches, &pb.AssistantMessageMetadata_WebSearch{
Query: search.Query,
})
}
for _, source := range metadata.Sources {
protoMetadata.Sources = append(protoMetadata.Sources, &pb.AssistantMessageMetadata_Source{
Url: source.URL,
})
}
return protoMetadata, string(payload)
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/chatservice/dao/dao_sqlite.go (1)
96-106:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject malformed
metadatabefore persisting it.These two inserts now define the SQLite storage contract for assistant metadata, but they still write any string verbatim. Since the history layer deserializes this field later, one bad payload here can poison the row and break metadata rendering on read. Trim and JSON-validate
metadatabefore both inserts.🛠️ Suggested guard
+func normalizeMetadata(metadata string) (string, error) { + trimmed := strings.TrimSpace(metadata) + if trimmed == "" { + return "", nil + } + + var tmp interface{} + if err := json.Unmarshal([]byte(trimmed), &tmp); err != nil { + return "", fmt.Errorf("invalid JSON format for metadata field: %w", err) + } + return trimmed, nil +}Call this from both write paths and persist the normalized value.
Also applies to: 175-268
🤖 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 `@backend/chatservice/dao/dao_sqlite.go` around lines 96 - 106, Reject and normalize malformed metadata before persisting by trimming whitespace and JSON-validating/parsing the metadata string, then re-serializing it (or storing null/empty) and using that normalized value in the INSERT; update the AddChatMessage method to perform this trim + JSON validation on the metadata argument and persist the normalized JSON string (or nil) instead of the raw input, and extract the same logic into a small helper (e.g., normalizeMetadata) and call it from both write paths (AddChatMessage and the other insert routine around lines 175-268) so both inserts persist the validated/normalized metadata value.backend/chatservice/dao/dao_postgres.go (1)
109-139:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
metadataJSON before either insert path writes it.This PR makes
metadatapart of the persisted message contract, but both write paths still accept and store arbitrary text. A malformed payload will be committed here and can break later history reads/source rendering when the service unmarshals it downstream. Reuse the same trim/json.Unmarshalguard you already apply toreferences.🛠️ Suggested guard
+func normalizeMetadata(metadata string) (string, error) { + trimmed := strings.TrimSpace(metadata) + if trimmed == "" { + return "", nil + } + + var tmp interface{} + if err := json.Unmarshal([]byte(trimmed), &tmp); err != nil { + return "", fmt.Errorf("invalid JSON format for metadata field: %w", err) + } + return trimmed, nil +}Apply that before both
INSERT INTO chat_messagescalls and persist the normalized value.Also applies to: 207-303
🤖 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 `@backend/chatservice/dao/dao_postgres.go` around lines 109 - 139, The metadata string is not validated before inserting, so add the same trim-and-JSON-validate guard used for references inside AddChatMessage (and the other insert path around lines 207-303): create a trimmedMetadata variable = strings.TrimSpace(metadata), set it to "{}" (or other normalized JSON empty object) if empty, attempt json.Unmarshal([]byte(trimmedMetadata), &temp) and return an error on failure, then use trimmedMetadata (the normalized, validated JSON) in the INSERT statements instead of the raw metadata; reference AddChatMessage and the alternate insert block to locate where to apply the change and reuse the same error logging pattern used for references.
🤖 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 `@backend/chatservice/service/agentic_chat.go`:
- Around line 380-383: The current handler appends BROWSER_SCRAPE_TOOL_NAME URLs
to sourceURLs on ToolCallStartEvent which records failed/cancelled scrapes as
sources; instead, when handling ToolCallStartEvent (in the agentic chat event
loop/handler in agentic_chat.go) add the URL to a temporary pending map/set
keyed by the tool call id (or event id), and only move that URL from pending
into sourceURLs from the tool success path (e.g., the ToolCallSuccessEvent
handler for the same tool call); also ensure the pending entry is removed on
failure/cancel paths so only successful scrapes become sources.
---
Outside diff comments:
In `@backend/chatservice/dao/dao_postgres.go`:
- Around line 109-139: The metadata string is not validated before inserting, so
add the same trim-and-JSON-validate guard used for references inside
AddChatMessage (and the other insert path around lines 207-303): create a
trimmedMetadata variable = strings.TrimSpace(metadata), set it to "{}" (or other
normalized JSON empty object) if empty, attempt
json.Unmarshal([]byte(trimmedMetadata), &temp) and return an error on failure,
then use trimmedMetadata (the normalized, validated JSON) in the INSERT
statements instead of the raw metadata; reference AddChatMessage and the
alternate insert block to locate where to apply the change and reuse the same
error logging pattern used for references.
In `@backend/chatservice/dao/dao_sqlite.go`:
- Around line 96-106: Reject and normalize malformed metadata before persisting
by trimming whitespace and JSON-validating/parsing the metadata string, then
re-serializing it (or storing null/empty) and using that normalized value in the
INSERT; update the AddChatMessage method to perform this trim + JSON validation
on the metadata argument and persist the normalized JSON string (or nil) instead
of the raw input, and extract the same logic into a small helper (e.g.,
normalizeMetadata) and call it from both write paths (AddChatMessage and the
other insert routine around lines 175-268) so both inserts persist the
validated/normalized metadata value.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d6642204-e7a2-40a2-8b2b-ba3552e5935b
⛔ Files ignored due to path filters (1)
backend/chatservice/proto/chatservice.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (11)
backend/chatservice/dao/dao.gobackend/chatservice/dao/dao_postgres.gobackend/chatservice/dao/dao_sqlite.gobackend/chatservice/dao/db/postgres/scripts/migrations/16_add_message_metadata.up.sqlbackend/chatservice/dao/db/sqlite/scripts/migrations/23_add_message_metadata.up.sqlbackend/chatservice/dao/models.gobackend/chatservice/service/agentic_chat.gobackend/chatservice/service/service.gofrontend/proto/chatservice.tsfrontend/src/pages/chat.tsxproto/chatservice.proto
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 `@docs/features/agentic_chat.md`:
- Line 95: Replace the unhyphenated compound adjective "assistant-message level"
with the hyphenated form "assistant-message-level" in the sentence that reads
"This column stores assistant-message level metadata as JSON." (search for that
exact phrase in docs/features/agentic_chat.md) so the compound adjective
correctly modifies "metadata".
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4d4a581-7e64-477b-8b13-756ba9f14f02
⛔ Files ignored due to path filters (1)
backend/chatservice/proto/chatservice.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (5)
backend/chatservice/service/agentic_chat.godocs/features/agentic_chat.mdfrontend/proto/chatservice.tsfrontend/src/pages/chat.tsxproto/chatservice.proto
✅ Files skipped from review due to trivial changes (1)
- frontend/proto/chatservice.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- proto/chatservice.proto
- backend/chatservice/service/agentic_chat.go
- frontend/src/pages/chat.tsx
Summary by CodeRabbit
New Features
Documentation