feat: add new design for messages and add inline references#2020
feat: add new design for messages and add inline references#2020lucaseduoli wants to merge 13 commits into
Conversation
…and chunk overlap
…gn for assistant message
WalkthroughThis PR propagates chunk metadata and parser labels through ingestion, indexing, search, and retrieval, then uses the enriched data to render citation-linked assistant messages with a chunk details popup and citation cards. ChangesCitation-aware chunk metadata flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AssistantMessage
participant MarkdownRenderer
participant ChunkPopup
User->>AssistantMessage: view assistant response
AssistantMessage->>MarkdownRenderer: render processed citation text
User->>MarkdownRenderer: click `#citation-N` link
MarkdownRenderer->>AssistantMessage: onCitationClick(index, anchorElement)
AssistantMessage->>ChunkPopup: open cited chunk details
ChunkPopup-->>User: display chunk metadata and source text
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
frontend/app/chat/_types/types.ts (1)
40-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a shared metadata type to reduce 4x field duplication.
embedding_model,parser,page,score,chunk_size,chunk_overlapare each declared four times (top-level,data,data.metadata,metadata). This mirrors the fallback chain inchunk-popup.tsx'sgetMetadataValue, so the shape duplication is functionally intentional, but keeping the field list in sync across four spots increases the chance of drift when new metadata fields are added.♻️ Suggested refactor
+interface ChunkMetadataFields { + embedding_model?: string; + parser?: string; + page?: number | string; + score?: number | string; + chunk_size?: number | string; + chunk_overlap?: number | string; +} + export interface ToolCallResult { text_key?: string; data?: { file_path?: string; text?: string; - page?: number | string; - score?: number | string; - embedding_model?: string; - parser?: string; - chunk_size?: number | string; - chunk_overlap?: number | string; - metadata?: { - embedding_model?: string; - parser?: string; - page?: number | string; - score?: number | string; - chunk_size?: number | string; - chunk_overlap?: number | string; - [key: string]: unknown; - }; + metadata?: ChunkMetadataFields & { [key: string]: unknown }; + [key: string]: unknown; + } & ChunkMetadataFields; + default_value?: string; + chunk_id?: string; + id?: string; + filename?: string; + source_url?: string | null; + text?: string; + metadata?: ChunkMetadataFields & { [key: string]: unknown }; + [key: string]: unknown; +}🤖 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 `@frontend/app/chat/_types/types.ts` around lines 40 - 84, The ToolCallResult shape repeats the same metadata fields in four places, which makes future updates easy to drift. Extract a shared metadata type for the common fields used by chunk-popup.tsx’s getMetadataValue fallback chain, then reuse it for ToolCallResult’s top-level, data, data.metadata, and metadata properties. Keep the overall structure the same, but centralize the repeated field list so new metadata fields only need to be added once.src/services/langflow_file_service.py (1)
476-478: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize parser label strings to avoid drift.
"Docling Serve 1.20.0"and"URL Ingester"are hardcoded here, whileprocessors.pydefinesDOCLING_PARSER_LABEL/TEXT_PARSER_LABELconstants for the same values, andchat_service.pyalso hardcodes"URL Ingester"in two places. Consider moving these labels into a shared module (e.g. alongside the existing constants inprocessors.py, or a newconstants.py) and importing them everywhere they're used, so a future rename/version bump doesn't require hunting down every literal.Also applies to: 648-650
🤖 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/services/langflow_file_service.py` around lines 476 - 478, The parser labels are duplicated as hardcoded strings, so centralize them in a shared constant location and reuse those symbols everywhere. Move or reuse the existing DOCLING_PARSER_LABEL and TEXT_PARSER_LABEL definitions from processors.py, and replace the inline “Docling Serve 1.20.0” and “URL Ingester” literals in langflow_file_service.py and chat_service.py with imports from that shared source. Keep the naming consistent across the related parser/service methods so future version or label changes only need one update.src/services/chat_service.py (1)
107-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
DocumentIndexContext/ingest-token construction acrosslangflow_chatandupload_context_chat.Both methods build an almost identical URL-ingest
DocumentIndexContext(now with the addedparser/chunk_size/chunk_overlapfields) and mint a token the same way. Extracting a small helper (e.g._build_url_ingest_token(...)) would prevent the two call sites from drifting as more fields are added.Also applies to: 524-545
🤖 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/services/chat_service.py` around lines 107 - 128, The URL-ingest `DocumentIndexContext` and token minting logic is duplicated in `langflow_chat` and `upload_context_chat`, so extract it into a shared helper such as `_build_url_ingest_token(...)` or a small builder used by both methods. Move the repeated `DocumentIndexContext` construction in `chat_service.py` into that helper, including the new `parser`, `chunk_size`, and `chunk_overlap` fields, and have both call sites reuse it to keep the two paths in sync.src/agent.py (1)
37-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffSystem prompt duplicated verbatim across three files.
The identical prompt string (with the new citation rule) is hardcoded in
src/agent.py(line 37),src/config/config_manager.py(AgentConfig.system_prompt), andfrontend/lib/constants.ts(DEFAULT_AGENT_SETTINGS.system_prompt). Any future change to citation formatting or tool instructions must be kept in sync across all three, which is error-prone.Consider deriving
agent.py's in-memory default fromconfig_manager.AgentConfig.system_prompt(single backend source of truth), and having the frontend fetch it from settings rather than hardcoding a copy.🤖 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/agent.py` at line 37, The system prompt text is duplicated across multiple places, so changes to tool instructions or citation formatting will drift out of sync. Make `src/agent.py` use the single backend source of truth from `AgentConfig.system_prompt` in `src/config/config_manager.py`, and remove the hardcoded frontend copy in `DEFAULT_AGENT_SETTINGS.system_prompt` by loading it from settings instead. Keep the prompt content centralized and referenced by `agent.py`, `AgentConfig`, and `DEFAULT_AGENT_SETTINGS` so there is only one canonical prompt to update.frontend/app/chat/_components/chunk-popup.tsx (1)
63-71: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winHardcoded fabricated parser version as fallback.
When no parser metadata is present, this falls back to a hardcoded, specific-sounding version string
"Docling Serve 1.20.0"rather than a generic label. UnlikeformatSplitConfig, which correctly falls back to actual settings values, this presents a guessed, potentially stale/incorrect version as if it were factual data.♻️ Proposed fix
const fileExt = filename.split(".").pop()?.toLowerCase() || ""; if (fileExt === "txt" || fileExt === "md") return "Text Parser"; - return "Docling Serve 1.20.0"; + return "Docling Serve";
🤖 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 `@frontend/app/chat/_components/assistant-message.tsx`:
- Around line 292-296: The filename derivation in assistant-message.tsx is
duplicated and diverges from citation-cards.tsx, so both consumers should use
the same shared helper. Extract the “derive display filename from data.file_path
or filename” logic into a common helper near the existing markdown-renderer.tsx
utilities (or another shared location), then update assistant-message.tsx and
citation-cards.tsx to call it so the fallback behavior and default label stay
consistent.
In `@frontend/app/chat/_components/chunk-popup.tsx`:
- Around line 176-247: The chunk popover in the popup component behaves like a
modal but is missing dialog semantics and an initial focus target. Update the
main container rendered by the chunk popup (the fixed motion.div in the chunk
popup component) to expose dialog accessibility metadata such as role="dialog"
and aria-modal="true", and ensure focus moves into the popup when isOpen becomes
true. Use a ref on the popup shell or the existing “Close chunk details” button
to set initial focus in the same chunk-popup component, and keep the Escape
handler wired through the existing onClose logic.
In `@frontend/app/chat/_components/citation-cards.tsx`:
- Around line 36-90: In the citation card rendering inside citedSources.map, the
page label only checks page !== null, so undefined still renders as “page
undefined”. Update the page handling to treat undefined as absent and, if
available, fall back to item.data?.page before displaying it. Keep the existing
filename fallback pattern consistent in the same component so the page text is
only shown when a real value exists.
In `@src/services/document_index_writer.py`:
- Around line 224-237: The `chunk_size`/`chunk_overlap` coercion in
`document_index_writer.py` is silently falling back to the raw value when
`int(value)` fails, which can create OpenSearch mapping conflicts. Update the
loop in the document-building logic so that invalid non-numeric values are
skipped instead of assigned to `doc[field_name]`, keeping the indexed type
consistent with the existing numeric mapping. Use the existing `context` and
`metadata` lookup flow around `parser`, `chunk_size`, and `chunk_overlap` to
locate the fix.
---
Nitpick comments:
In `@frontend/app/chat/_types/types.ts`:
- Around line 40-84: The ToolCallResult shape repeats the same metadata fields
in four places, which makes future updates easy to drift. Extract a shared
metadata type for the common fields used by chunk-popup.tsx’s getMetadataValue
fallback chain, then reuse it for ToolCallResult’s top-level, data,
data.metadata, and metadata properties. Keep the overall structure the same, but
centralize the repeated field list so new metadata fields only need to be added
once.
In `@src/agent.py`:
- Line 37: The system prompt text is duplicated across multiple places, so
changes to tool instructions or citation formatting will drift out of sync. Make
`src/agent.py` use the single backend source of truth from
`AgentConfig.system_prompt` in `src/config/config_manager.py`, and remove the
hardcoded frontend copy in `DEFAULT_AGENT_SETTINGS.system_prompt` by loading it
from settings instead. Keep the prompt content centralized and referenced by
`agent.py`, `AgentConfig`, and `DEFAULT_AGENT_SETTINGS` so there is only one
canonical prompt to update.
In `@src/services/chat_service.py`:
- Around line 107-128: The URL-ingest `DocumentIndexContext` and token minting
logic is duplicated in `langflow_chat` and `upload_context_chat`, so extract it
into a shared helper such as `_build_url_ingest_token(...)` or a small builder
used by both methods. Move the repeated `DocumentIndexContext` construction in
`chat_service.py` into that helper, including the new `parser`, `chunk_size`,
and `chunk_overlap` fields, and have both call sites reuse it to keep the two
paths in sync.
In `@src/services/langflow_file_service.py`:
- Around line 476-478: The parser labels are duplicated as hardcoded strings, so
centralize them in a shared constant location and reuse those symbols
everywhere. Move or reuse the existing DOCLING_PARSER_LABEL and
TEXT_PARSER_LABEL definitions from processors.py, and replace the inline
“Docling Serve 1.20.0” and “URL Ingester” literals in langflow_file_service.py
and chat_service.py with imports from that shared source. Keep the naming
consistent across the related parser/service methods so future version or label
changes only need one update.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f0be1b9c-eec0-4b2a-808c-c6d0d230a818
📒 Files selected for processing (29)
flows/components/opensearch_multimodal.pyflows/ingestion_flow.jsonflows/openrag_agent.jsonflows/openrag_nudges.jsonflows/openrag_url_mcp.jsonfrontend/app/api/mutations/useUpdateOnboardingStateMutation.tsfrontend/app/api/queries/useGetSearchQuery.tsfrontend/app/api/queries/useGetSettingsQuery.tsfrontend/app/chat/_components/assistant-message.tsxfrontend/app/chat/_components/chunk-popup.tsxfrontend/app/chat/_components/citation-cards.tsxfrontend/app/chat/_components/function-calls/result.tsxfrontend/app/chat/_components/message-actions.tsxfrontend/app/chat/_components/message.tsxfrontend/app/chat/_types/types.tsfrontend/app/globals.cssfrontend/app/onboarding/_components/onboarding-content.tsxfrontend/components/markdown-renderer.tsxfrontend/lib/constants.tsfrontend/tailwind.config.tssrc/agent.pysrc/api/settings/models.pysrc/config/config_manager.pysrc/models/processors.pysrc/services/chat_service.pysrc/services/document_index_writer.pysrc/services/langflow_file_service.pysrc/services/langflow_ingest_token_service.pysrc/services/search_service.py
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 5 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/components/markdown-renderer.tsx (1)
122-137: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueCitation click handling looks correct; minor NaN edge case.
parseInt(href.replace("#citation-", ""), 10)has no validation, so a malformed#citation-href (e.g., non-numeric suffix) would callonCitationClick?.(NaN, ...). Since these hrefs are self-generated inpreprocessCitationsas#citation-${index}, this is unlikely to occur in practice.🤖 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 `@frontend/components/markdown-renderer.tsx` around lines 122 - 137, The citation link handler in the markdown renderer should guard against malformed `#citation-` href values before calling onCitationClick. Update the a() branch in markdown-renderer.tsx to validate the parsed index from href (after stripping the `#citation-` prefix) and only render the button/call onCitationClick when the value is a valid number; otherwise fall back gracefully to the normal link behavior or a no-op path.
🤖 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 `@frontend/components/markdown-citations.ts`:
- Around line 45-76: The citation replacement in markdown-citations.ts is
dropping unmatched `(Source: ...)`/`[Source: ...]` markers because the replace
callback returns an empty string when no ids resolve in sourceLookup. Update the
text.replace handling in the citation processing logic to preserve the original
marker text, or emit a neutral fallback, when replacementBadges stays empty.
Keep the existing unique matching flow using sourceLookup, citedSourcesMap, and
citedSourcesList, but avoid silently removing unresolved citations.
- Around line 8-27: The lookup built in buildSourceLookup/addLookupKey is too
lossy because data?.file_path and filename can map multiple chunks to one
ToolCallResult, causing incorrect citation matching. Update markdown-citations
so chunk_id and id remain the primary keys, but treat file_path/filename
fallback lookups as unambiguous only, or keep them separate instead of
overwriting entries in the same Map. Make the change in buildSourceLookup and
the helper used by addLookupKey so citations resolve to the correct chunk.
---
Nitpick comments:
In `@frontend/components/markdown-renderer.tsx`:
- Around line 122-137: The citation link handler in the markdown renderer should
guard against malformed `#citation-` href values before calling onCitationClick.
Update the a() branch in markdown-renderer.tsx to validate the parsed index from
href (after stripping the `#citation-` prefix) and only render the button/call
onCitationClick when the value is a valid number; otherwise fall back gracefully
to the normal link behavior or a no-op path.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: c9195540-6119-4027-ba99-256e0308dab8
📒 Files selected for processing (5)
frontend/app/chat/_components/assistant-message.tsxfrontend/app/chat/_components/chunk-popup.tsxfrontend/app/chat/_components/citation-cards.tsxfrontend/components/markdown-citations.tsfrontend/components/markdown-renderer.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/app/chat/_components/citation-cards.tsx
- frontend/app/chat/_components/chunk-popup.tsx
- frontend/app/chat/_components/assistant-message.tsx
| const addLookupKey = ( | ||
| sourceLookup: Map<string, ToolCallResult>, | ||
| key: string | undefined, | ||
| source: ToolCallResult, | ||
| ) => { | ||
| if (key) sourceLookup.set(key, source); | ||
| }; | ||
|
|
||
| const buildSourceLookup = (sources: ToolCallResult[]) => { | ||
| const sourceLookup = new Map<string, ToolCallResult>(); | ||
|
|
||
| for (const source of sources) { | ||
| addLookupKey(sourceLookup, source.chunk_id, source); | ||
| addLookupKey(sourceLookup, source.id, source); | ||
| addLookupKey(sourceLookup, source.data?.file_path, source); | ||
| addLookupKey(sourceLookup, source.filename, source); | ||
| } | ||
|
|
||
| return sourceLookup; | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and nearby context
git ls-files frontend/components/markdown-citations.ts
wc -l frontend/components/markdown-citations.ts
cat -n frontend/components/markdown-citations.ts | sed -n '1,220p'
# Find citation lookup/resolution call sites and related types
rg -n "buildSourceLookup|addLookupKey|chunk_id|file_path|filename|markdown-citations|citation" frontend -S
# Inspect the ToolCallResult type definition if present in frontend
rg -n "type ToolCallResult|interface ToolCallResult|export .*ToolCallResult" frontend -SRepository: langflow-ai/openrag
Length of output: 26315
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect ToolCallResult and the chat code that emits citation markers / source blocks
cat -n frontend/app/chat/_types/types.ts | sed -n '1,220p'
sed -n '180,260p' frontend/app/chat/page.tsx | cat -n
sed -n '1,180p' frontend/app/chat/_components/citation-cards.tsx | cat -n
sed -n '150,230p' frontend/components/markdown-renderer.tsx | cat -n
# Look for any code that generates [Source: ...] or (Source: ...)
rg -n "\[Source:|\(Source:|Source:" frontend/app frontend/components frontend/lib -S
# Inspect the backend-facing prompt/instructions that mention citation format
sed -n '1,140p' frontend/lib/constants.ts | cat -nRepository: langflow-ai/openrag
Length of output: 23370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Trace how preprocessCitations is used and whether filename/file_path citations are expected
rg -n "preprocessCitations\(|citedSources|citation-" frontend -S
sed -n '1,120p' frontend/app/chat/_components/assistant-message.tsx | cat -n
sed -n '320,390p' frontend/lib/chat-stream-parsers.ts | cat -nRepository: langflow-ai/openrag
Length of output: 8623
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search the whole repo for concrete citation examples that use filenames or file paths
rg -n "\[Source:\s*[^\]]*(filename|file_path|document)|\(Source:\s*[^(]*(filename|file_path|document)" . -S
# Search for any prompt/docs that instruct filename-based citations
rg -n "cite.*filename|filename.*cite|file_path.*cite|Source:.*filename|Source:.*document" . -S
# Inspect the markdown citation helper for exact matching behavior around source markers
sed -n '1,120p' frontend/components/markdown-citations.ts | cat -nRepository: langflow-ai/openrag
Length of output: 22125
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the backend citation parser around the filename marker handling
sed -n '660,710p' src/agent.py | cat -nRepository: langflow-ai/openrag
Length of output: 2682
Avoid lossy filename/file_path fallback lookups. chunk_id/id are fine, but data?.file_path and filename collapse all chunks in a document to one map entry, so a (Source: filename) marker can attach to the wrong chunk when multiple chunks share that key. Keep those fallback keys separate or only use them when the match is unambiguous.
🤖 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 `@frontend/components/markdown-citations.ts` around lines 8 - 27, The lookup
built in buildSourceLookup/addLookupKey is too lossy because data?.file_path and
filename can map multiple chunks to one ToolCallResult, causing incorrect
citation matching. Update markdown-citations so chunk_id and id remain the
primary keys, but treat file_path/filename fallback lookups as unambiguous only,
or keep them separate instead of overwriting entries in the same Map. Make the
change in buildSourceLookup and the helper used by addLookupKey so citations
resolve to the correct chunk.
| const processedText = text.replace(regex, (_match, p1, p2) => { | ||
| const rawIds = p1 || p2; | ||
| if (!rawIds) return ""; | ||
|
|
||
| const ids = rawIds.split(",").map((id: string) => id.trim()); | ||
| const replacementBadges: string[] = []; | ||
|
|
||
| for (const rawId of ids) { | ||
| const foundSource = sourceLookup.get(rawId); | ||
|
|
||
| if (foundSource) { | ||
| const uniqueKey = (foundSource.chunk_id || | ||
| foundSource.id || | ||
| foundSource.filename || | ||
| JSON.stringify(foundSource)) as string; | ||
|
|
||
| let index = citedSourcesMap.get(uniqueKey); | ||
| if (index === undefined) { | ||
| index = nextIndex++; | ||
| citedSourcesMap.set(uniqueKey, index); | ||
| citedSourcesList.push({ item: foundSource, index }); | ||
| } | ||
| replacementBadges.push(`[\\[${index}\\]](#citation-${index})`); | ||
| } | ||
| } | ||
|
|
||
| if (replacementBadges.length > 0) { | ||
| return replacementBadges.join(""); | ||
| } | ||
|
|
||
| return ""; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Unmatched citation markers vanish silently.
When none of the ids in a (Source: ...)/[Source: ...] marker resolve via sourceLookup, the whole match is replaced with "" (Line 75), removing the marker without any fallback or indication. If backend id formats ever drift from frontend expectations, citations will disappear from responses with no visible symptom.
Consider leaving the original text intact (or a neutral marker) when no source is found, to aid debugging and avoid silently hiding content.
🤖 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 `@frontend/components/markdown-citations.ts` around lines 45 - 76, The citation
replacement in markdown-citations.ts is dropping unmatched `(Source:
...)`/`[Source: ...]` markers because the replace callback returns an empty
string when no ids resolve in sourceLookup. Update the text.replace handling in
the citation processing logic to preserve the original marker text, or emit a
neutral fallback, when replacementBadges stays empty. Keep the existing unique
matching flow using sourceLookup, citedSourcesMap, and citedSourcesList, but
avoid silently removing unresolved citations.
Fixed 5 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <[email protected]>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/components/markdown-renderer.tsx (1)
61-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
deriveDisplayFilenameis duplicated verbatim in 3 files.The exact same implementation is also defined independently in
frontend/app/chat/_components/assistant-message.tsx(lines 65-72) andfrontend/app/chat/_components/citation-cards.tsx(lines 65-72), rather than importing it from this file. Any future fix or behavior change (e.g., the fallback-on-trailing-slash edge case) now needs to be applied in three places and will silently drift.Consider moving this helper into a shared utility module (e.g.
frontend/lib/citations.ts) and importing it frommarkdown-renderer.tsx,citation-cards.tsx, andassistant-message.tsx.♻️ Suggested consolidation
-export const deriveDisplayFilename = ( - filePath: string | undefined, - filename: string | undefined, - fallback: string = "Document", -): string => { - const path = filePath || filename || fallback; - return path.split("/").pop() || path; -}; +export { deriveDisplayFilename } from "`@/lib/citations`";🤖 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 `@frontend/components/markdown-renderer.tsx` around lines 61 - 72, The `deriveDisplayFilename` helper is duplicated across `markdown-renderer.tsx`, `assistant-message.tsx`, and `citation-cards.tsx`, so consolidate it into a shared utility module and import it from all three places. Move the single implementation to a common citation utility (for example, a shared helper alongside citation logic), then replace the local copies with imports so future filename/fallback fixes only need to be made once.
🤖 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 `@frontend/components/markdown-renderer.tsx`:
- Around line 61-72: The `deriveDisplayFilename` helper is duplicated across
`markdown-renderer.tsx`, `assistant-message.tsx`, and `citation-cards.tsx`, so
consolidate it into a shared utility module and import it from all three places.
Move the single implementation to a common citation utility (for example, a
shared helper alongside citation logic), then replace the local copies with
imports so future filename/fallback fixes only need to be made once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f045d360-8d5b-46c3-ae14-43857c193616
📒 Files selected for processing (5)
frontend/app/chat/_components/assistant-message.tsxfrontend/app/chat/_components/chunk-popup.tsxfrontend/app/chat/_components/citation-cards.tsxfrontend/components/markdown-renderer.tsxsrc/services/document_index_writer.py
🚧 Files skipped from review as they are similar to previous changes (4)
- src/services/document_index_writer.py
- frontend/app/chat/_components/assistant-message.tsx
- frontend/app/chat/_components/chunk-popup.tsx
- frontend/app/chat/_components/citation-cards.tsx
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The branch was updated while autofix was in progress. Please try again. |
ricofurtado
left a comment
There was a problem hiding this comment.
The onboarding-content line 99 comment is the one that got my attention the other are medium
| return path.split("/").pop() || path; | ||
| }; | ||
|
|
||
| const buildSourceLookup = (sources: ToolCallResult[]) => { |
There was a problem hiding this comment.
maps file_path and filename into a single Map, so duplicate filenames overwrite earlier chunks. If old/custom prompts cite a filename instead of chunk_id, clicking that citation can open an arbitrary last chunk from the same file. Keep chunk_id/id primary, and only use filename/file_path fallback when unambiguous.
| // Patterns: (Source: chunk_id) or [Source: chunk_id] | ||
| const regex = /\[Source:\s*([^\]]+)\]|\(Source:\s*([^)]+)\)/g; | ||
|
|
||
| const processedText = text.replace(regex, (_match, p1, p2) => { |
There was a problem hiding this comment.
replaces (Source: ...) markers, but when no source resolves it returns "" at line 88. If the model cites an ID missing from functionCalls, the UI removes the citation marker and leaves the factual claim looking uncited. Better to preserve the original marker or render an “unresolved source” badge so grounding failures remain visible.
| }); | ||
| setAssistantMessage(message); | ||
| // Save assistant message to backend | ||
| await updateOnboardingMutation.mutateAsync({ |
There was a problem hiding this comment.
onboarding now persists full retrieval results into workspace config
onboarding-content.tsx (line 100) sends message.functionCalls to /api/settings, models.py (line 57) accepts arbitrary function-call dicts, and config_manager.py (line 488) writes them into onboarding config. Those function calls can contain retrieved chunk text, filenames, source URLs, scores, etc. That means RAG source content can be copied into workspace config / workspace_config, outside the normal chat storage and retention path. I’d strip functionCalls before persisting onboarding state, or persist only minimal citation metadata needed for display.
This pull request introduces significant improvements to the chat assistant's user experience by adding detailed popovers for retrieved search result "chunks" (citations) and enhancing metadata handling across both frontend and backend. The main changes include new UI components for chunk details, improved citation handling in assistant messages, and expanded metadata support throughout the data flow.
Enhanced Citation Display and Chunk Details:
ChunkPopupcomponent that displays detailed information about a cited chunk, including source text, parser, chunking configuration, embedding model, score, and a link to the original document. This popover appears when a user clicks on a citation in the assistant message.AssistantMessageto support citation click handling, manage popover state, preprocess citations, and renderCitationCardsand the newChunkPopupfor interactive chunk exploration. [1] [2] [3] [4] [5] [6]Improved Metadata Flow and Types:
opensearch_multimodal.py) and frontend (useGetSearchQuery.ts, related types) to include additional metadata fields for each chunk:parser,chunk_size, andchunk_overlap, ensuring this information is available for display in the UI. [1] [2]chunk_id,id, andscorefields for more precise chunk identification and scoring in the UI.Type and API Consistency:
These changes collectively provide users with a richer, more interactive experience when reviewing AI-generated responses and their supporting sources.
Most Important Changes:
1. User Interface Enhancements
ChunkPopupcomponent for detailed chunk/citation popovers, including metadata and source text, accessible from assistant messages.AssistantMessageto support citation click events, manage popover state, preprocess citations, and renderCitationCardsandChunkPopup. [1] [2] [3] [4] [5] [6]2. Metadata and Backend Support
parser,chunk_size, andchunk_overlapfields for each chunk, and addedchunk_id,id, andscoreto chunk metadata. [1] [2]3. TypeScript Types and API Consistency
Summary by CodeRabbit
Summary of changes
(Source: <chunk_id>)rules.