feat: construct resource url from backend #98
Conversation
|
WalkthroughThe recent updates introduce a new Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- templates/types/streaming/express/src/controllers/stream-helper.ts (2 hunks)
- templates/types/streaming/fastapi/app/api/routers/chat.py (2 hunks)
- templates/types/streaming/nextjs/app/api/chat/stream-helper.ts (2 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/chat-sources.tsx (4 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/index.ts (1 hunks)
Additional Context Used
Biome (18)
templates/types/streaming/express/src/controllers/stream-helper.ts (4)
21-21: The computed expression can be simplified without the use of a string literal.
23-23: The computed expression can be simplified without the use of a string literal.
1-1: All these imports are only used as types.
1-8: Some named imports are only used as types.
templates/types/streaming/nextjs/app/api/chat/stream-helper.ts (4)
21-21: The computed expression can be simplified without the use of a string literal.
23-23: The computed expression can be simplified without the use of a string literal.
1-1: All these imports are only used as types.
1-8: Some named imports are only used as types.
templates/types/streaming/nextjs/app/components/ui/chat/chat-sources.tsx (9)
20-20: This enum declaration contains members that are implicitly initialized.
34-34: The computed expression can be simplified without the use of a string literal.
42-42: The computed expression can be simplified without the use of a string literal.
43-43: The computed expression can be simplified without the use of a string literal.
64-73: Prefer for...of instead of forEach.
90-90: Forbidden non-null assertion.
122-122: Avoid using target="_blank" without rel="noreferrer".
126-126: Forbidden non-null assertion.
4-5: All these imports are only used as types.
templates/types/streaming/nextjs/app/components/ui/chat/index.ts (1)
1-1: All these imports are only used as types.
Additional comments not posted (5)
templates/types/streaming/nextjs/app/components/ui/chat/index.ts (1)
24-24: The addition of the optionalurlfield to theSourceNodetype is well-integrated and enhances URL handling capabilities.templates/types/streaming/express/src/controllers/stream-helper.ts (1)
20-29: ThegetNodeUrlfunction is well-implemented, providing a flexible way to determine node URLs based on available metadata.templates/types/streaming/nextjs/app/api/chat/stream-helper.ts (1)
20-29: The implementation ofgetNodeUrlin the Next.js template is consistent with the Express template, ensuring uniform functionality across platforms.templates/types/streaming/nextjs/app/components/ui/chat/chat-sources.tsx (1)
Line range hint
38-49: The updates togetNodeInfofunction to usenode.urldirectly simplify URL handling and align with the PR's objectives.templates/types/streaming/fastapi/app/api/routers/chat.py (1)
42-60: The updates to the_SourceNodesclass to handle an optionalurlfield and the enhancements in URL construction are well-implemented and align with the PR's objectives.
| @@ -33,12 +32,11 @@ type NodeInfo = { | |||
|
|
|||
| function getNodeInfo(node: SourceNode): NodeInfo { | |||
| if (typeof node.metadata["URL"] === "string") { | |||
There was a problem hiding this comment.
this code can be simplified as we render NODE_TYPE.URL and NODE_TYPE.FILE the same way, we can merge them - I guess we also don't need to check the metadata here and probably even can remove path
| if (url) return url; | ||
| const fileName = metadata["file_name"]; | ||
| if (fileName) { | ||
| const fileServerUrlPrefix = process.env.FILESERVER_URL_PREFIX || ""; |
There was a problem hiding this comment.
if process.env.FILESERVER_URL_PREFIX is not set, how about logging a warning and return undefined? (also in python)
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (1)
templates/components/engines/typescript/agent/tools/interpreter.ts (1)
Line range hint
3-3: Import Node.js built-in modules using thenode:protocol for clarity and consistency.- import crypto from "crypto"; + import crypto from "node:crypto"; - import path from "path"; + import path from "node:path";
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- templates/components/engines/typescript/agent/tools/interpreter.ts (2 hunks)
- templates/types/streaming/express/src/controllers/stream-helper.ts (2 hunks)
- templates/types/streaming/fastapi/app/api/routers/chat.py (2 hunks)
- templates/types/streaming/nextjs/app/api/chat/stream-helper.ts (2 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/chat-sources.tsx (5 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/widgets/PdfDialog.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- templates/types/streaming/fastapi/app/api/routers/chat.py
Additional Context Used
Biome (17)
templates/components/engines/typescript/agent/tools/interpreter.ts (3)
3-3: A Node.js builtin module should be imported with the node: protocol.
1-1: Some named imports are only used as types.
3-4: All these imports are only used as types.
templates/types/streaming/express/src/controllers/stream-helper.ts (4)
21-21: The computed expression can be simplified without the use of a string literal.
23-23: The computed expression can be simplified without the use of a string literal.
1-1: All these imports are only used as types.
1-8: Some named imports are only used as types.
templates/types/streaming/nextjs/app/api/chat/stream-helper.ts (4)
21-21: The computed expression can be simplified without the use of a string literal.
23-23: The computed expression can be simplified without the use of a string literal.
1-1: All these imports are only used as types.
1-8: Some named imports are only used as types.
templates/types/streaming/nextjs/app/components/ui/chat/chat-sources.tsx (5)
29-41: Prefer for...of instead of forEach.
58-58: Forbidden non-null assertion.
89-89: Avoid using target="_blank" without rel="noreferrer".
93-93: Forbidden non-null assertion.
4-5: All these imports are only used as types.
templates/types/streaming/nextjs/app/components/ui/chat/widgets/PdfDialog.tsx (1)
32-32: Avoid using target="_blank" without rel="noreferrer".
Additional comments not posted (2)
templates/types/streaming/express/src/controllers/stream-helper.ts (1)
24-28: Handle the case whereFILESERVER_URL_PREFIXis not set by logging a warning and returningundefined. This is a good practice to inform the user of missing environment configurations.templates/types/streaming/nextjs/app/api/chat/stream-helper.ts (1)
21-23: Consider simplifying the metadata access for clarity and consistency, as suggested in the previous review.
templates/types/streaming/nextjs/app/components/ui/chat/widgets/PdfDialog.tsx
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/components/ui/chat/chat-sources.tsx
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/components/ui/chat/chat-sources.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- templates/types/streaming/fastapi/app/api/routers/chat.py (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- templates/types/streaming/fastapi/app/api/routers/chat.py
| if not url_prefix: | ||
| logger.warning("Warning: FILESERVER_URL_PREFIX not set in environment variables") | ||
| if file_name and url_prefix: | ||
| url = f"{url_prefix}/data/{file_name}" |
There was a problem hiding this comment.
@leehuwuj don't we have data configurable (or in a constant)?
| return undefined; | ||
| } | ||
| if (fileName) { | ||
| return `${process.env.FILESERVER_URL_PREFIX}/data/${fileName}`; |
| return undefined; | ||
| } | ||
| if (fileName) { | ||
| return `${process.env.FILESERVER_URL_PREFIX}/data/${fileName}`; |
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
UI Changes