chore: import wavefront client and server #176
Conversation
WalkthroughAdds a new Wavefront web client: project scaffold, Docker builds, CI workflow, Express runtime server, global styles, extensive React UI primitives and components, typed Axios API services, React Query hooks/mutations, provider configs, and numerous asset/icon files. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: "Checkout" | ||
| uses: "actions/checkout@v3" | ||
|
|
||
| - name: Get commit hash | ||
| id: get-commit-hash | ||
| run: echo "::set-output name=commit-hash::$(git rev-parse --short HEAD)" | ||
|
|
||
| - name: Get timestamp | ||
| id: get-timestamp | ||
| run: echo "::set-output name=timestamp::$(date +'%Y-%m-%d-%H-%M')" | ||
|
|
||
| - name: Cache Docker layers | ||
| id: cache-docker-layers | ||
| uses: actions/cache@v3 | ||
| with: | ||
| path: /tmp/.buildx-cache | ||
| key: ${{ runner.os }}-docker-${{ github.sha }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-docker- | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 | ||
|
|
||
| - name: Build Docker Image | ||
| id: build-image | ||
| run: | | ||
| docker build -f wavefront/client/Dockerfile -t rootflo:${{ steps.get-commit-hash.outputs.commit-hash }}-${{ steps.get-timestamp.outputs.timestamp }} . | ||
| echo "IMAGE_TAG=${{ steps.get-commit-hash.outputs.commit-hash }}-${{ steps.get-timestamp.outputs.timestamp }}" >> $GITHUB_ENV | ||
|
|
||
| - id: "Auth-to-GCP" | ||
| uses: "google-github-actions/auth@v1" | ||
| with: | ||
| credentials_json: "${{ secrets.GCP_SERVICE_ACCOUNT_KEY }}" | ||
|
|
||
| - name: "Set up Cloud SDK" | ||
| uses: "google-github-actions/setup-gcloud@v1" | ||
|
|
||
| - name: "Docker auth for GCP" | ||
| run: |- | ||
| gcloud auth configure-docker ${{ env.GCP_REGION }}-docker.pkg.dev --quiet | ||
|
|
||
| - name: Tag and push image to GCP Artifact Registry | ||
| run: | | ||
| docker tag rootflo:${{ env.IMAGE_TAG }} ${{ env.GAR_LOCATION }}/${{ env.IMAGE_NAME }}:${{ env.IMAGE_TAG }} | ||
| docker push ${{ env.GAR_LOCATION }}/${{ env.IMAGE_NAME }}:${{ env.IMAGE_TAG }} | ||
|
|
||
| - name: Cleanup Docker images | ||
| run: | | ||
| docker rmi rootflo:${{ env.IMAGE_TAG }} || true | ||
| docker rmi ${{ env.GAR_LOCATION }}/${{ env.IMAGE_NAME }}:${{ env.IMAGE_TAG }} || true |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the problem, you should add a permissions block following the principle of least privilege. This allows the workflow or job to run only with the minimal GitHub token permissions required, preventing unauthorized or accidental write operations to the repository. As none of the jobs in the provided workflow require write access to repository contents or metadata, the minimal setting is appropriate: at the workflow or job level, add permissions: contents: read. The block can be added after the name: line at the top (root-level, affecting all jobs), or directly in the build-push-artifact job block if more granular control is desired. The recommendation is to add it at the root for maximum coverage and clarity. No imports or special methods are needed.
| @@ -1,4 +1,6 @@ | ||
| name: (Develop) Build and Push Wavefront Web app to AWS and GCP | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| workflow_dispatch: |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| logger.error(f'{error_msg} with error as {str(error)}') | ||
| return JSONResponse( | ||
| status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, | ||
| content=ResponseFormatter.buildErrorResponse(str(error)), |
| logger.error(f'{error_msg} with error as {str(error)}') | ||
| return JSONResponse( | ||
| status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, | ||
| content=ResponseFormatter.buildErrorResponse(str(error)), |
| logger.error(f'{error_msg} with error as {str(error)}') | ||
| return JSONResponse( | ||
| status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, | ||
| content=ResponseFormatter.buildErrorResponse(str(error)), |
wavefront/server/packages/flo_utils/flo_utils/legacy_db_repository/legacy_redshift.py
Dismissed
Show dismissed
Hide dismissed
...t/server/modules/image_search_module/image_search_module/services/reference_image_service.py
Dismissed
Show dismissed
Hide dismissed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
♻️ Duplicate comments (4)
wavefront/client/src/assets/icons/datasources.tsx (1)
1-14: Same React import verification needed.This component has the same TypeScript type reference without React import as noted in ai-agent-icon.tsx.
wavefront/client/src/assets/icons/model-repository-icon.tsx (1)
1-13: Same React import verification needed.This component has the same TypeScript type reference without React import as noted in ai-agent-icon.tsx.
wavefront/client/src/assets/icons/workflow-icon.tsx (1)
1-18: Same React import verification needed.This component has the same TypeScript type reference without React import as noted in ai-agent-icon.tsx.
wavefront/client/src/assets/icons/rag-icon.tsx (1)
1-12: Same React import verification needed.This component has the same TypeScript type reference without React import as noted in ai-agent-icon.tsx.
🟠 Major comments (16)
wavefront/client/src/assets/icons/api-icon.tsx-13-23 (1)
13-23: Add props parameter for consistency and reusability.
ApiActiveIconshould also accept props for consistency with other icons in this set.Apply this diff:
-const ApiActiveIcon = () => ( - <svg xmlns="http://www.w3.org/2000/svg" width="14" height="14" fill="none" viewBox="0 0 14 14"> +const ApiActiveIcon = ({ ...props }: React.SVGProps<SVGSVGElement>) => ( + <svg xmlns="http://www.w3.org/2000/svg" width="14" height="14" fill="none" viewBox="0 0 14 14" {...props}>wavefront/client/src/assets/icons/api-icon.tsx-1-11 (1)
1-11: Add props parameter for consistency and reusability.
ApiIcondoesn't accept props, unlike other icons in this set (e.g.,AiAgentIcon,DatasourcesIcon). This limits reusability.Apply this diff:
-const ApiIcon = () => ( - <svg xmlns="http://www.w3.org/2000/svg" width="14" height="14" fill="none" viewBox="0 0 14 14"> +const ApiIcon = ({ ...props }: React.SVGProps<SVGSVGElement>) => ( + <svg xmlns="http://www.w3.org/2000/svg" width="14" height="14" fill="none" viewBox="0 0 14 14" {...props}>wavefront/client/src/assets/icons/permission-icon.tsx-1-22 (1)
1-22: Add props parameter and use implicit return for consistency.
PermissionIcondoesn't accept props, unlike other icons in this set. This limits reusability and is inconsistent with icons likeAiAgentIcon,DatasourcesIcon, etc.Apply this diff:
-const PermissionIcon = () => { - return ( - <svg xmlns="http://www.w3.org/2000/svg" width="13" height="14" fill="none" viewBox="0 0 13 14"> +const PermissionIcon = ({ ...props }: React.SVGProps<SVGSVGElement>) => ( + <svg xmlns="http://www.w3.org/2000/svg" width="13" height="14" fill="none" viewBox="0 0 13 14" {...props}> <g id="vuesax/linear/shield-security"> <g id="shield-security" stroke="#22A622" strokeLinecap="round" strokeLinejoin="round" strokeWidth="1.1"> <path id="Vector" d="M5.682 1.708 2.98 2.726c-.623.233-1.132.97-1.132 1.63v4.025c0 .64.423 1.479.937 1.864l2.33 1.738c.763.575 2.02.575 2.783 0l2.33-1.738c.514-.385.937-1.225.937-1.864V4.357c0-.667-.51-1.403-1.132-1.636L7.329 1.708c-.46-.168-1.197-.168-1.647 0" ></path> <path id="Vector_2" strokeMiterlimit="10" d="M6.5 7.271a1.083 1.083 0 1 0 0-2.167 1.083 1.083 0 0 0 0 2.167" ></path> <path id="Vector_3" strokeMiterlimit="10" d="M6.5 7.27v1.626"></path> </g> </g> </svg> - ); -}; +);wavefront/client/src/assets/icons/phone-icon.tsx-10-19 (1)
10-19: Refactor for consistency: accept props and use implicit return.
PhoneActiveIconshould accept props for consistency with other icons and can use implicit return for cleaner code.Apply this diff:
-const PhoneActiveIcon = () => { - return ( - <svg xmlns="http://www.w3.org/2000/svg" width="14" height="14" fill="none" viewBox="0 0 14 14"> +const PhoneActiveIcon = ({ ...props }: React.SVGProps<SVGSVGElement>) => ( + <svg xmlns="http://www.w3.org/2000/svg" width="14" height="14" fill="none" viewBox="0 0 14 14" {...props}> <path fill="#0080E2" d="M6.446 8.72 5.366 9.8a.586.586 0 0 1-.822.006c-.064-.064-.128-.123-.192-.187a16.6 16.6 0 0 1-1.628-1.907 10.4 10.4 0 0 1-1.143-1.99q-.413-.996-.414-1.907c0-.397.07-.776.21-1.126q.208-.535.67-.974c.374-.367.782-.548 1.214-.548.163 0 .327.035.472.105a.95.95 0 0 1 .391.326l1.354 1.908c.105.146.18.28.233.408a.9.9 0 0 1 .082.356q0 .211-.123.414a2 2 0 0 1-.327.414L4.9 5.56a.31.31 0 0 0-.093.234.5.5 0 0 0 .017.134c.018.046.035.081.047.116q.158.291.542.747.393.456.846.922c.059.058.123.116.181.175.233.227.24.6.006.834M12.816 10.693c0 .163-.03.332-.088.495q-.024.07-.058.14c-.1.21-.227.409-.397.595a2.6 2.6 0 0 1-.956.689c-.006 0-.012.005-.018.005-.344.14-.717.216-1.12.216q-.894.001-1.901-.425-1.008-.429-2.007-1.155c-.228-.17-.455-.339-.67-.52l1.907-1.907q.246.183.431.28c.03.011.064.029.105.046a.4.4 0 0 0 .146.024.32.32 0 0 0 .24-.1l.443-.437a1.8 1.8 0 0 1 .42-.326.8.8 0 0 1 .414-.123q.164-.002.356.076.19.078.408.227l1.93 1.371a.9.9 0 0 1 .322.373c.058.146.093.292.093.456" ></path> </svg> - ); -}; +);wavefront/client/src/assets/icons/phone-icon.tsx-1-9 (1)
1-9: Add props parameter for consistency and reusability.
PhoneIcondoesn't accept props, unlike other icons in this set (e.g.,AiAgentIcon,DatasourcesIcon). This limits the ability to pass common SVG attributes likeclassName,onClick,aria-label, etc.Apply this diff to accept and spread props:
-const PhoneIcon = () => ( - <svg xmlns="http://www.w3.org/2000/svg" width="14" height="14" fill="none" viewBox="0 0 14 14"> +const PhoneIcon = ({ ...props }: React.SVGProps<SVGSVGElement>) => ( + <svg xmlns="http://www.w3.org/2000/svg" width="14" height="14" fill="none" viewBox="0 0 14 14" {...props}>wavefront/client/server.cjs-50-68 (1)
50-68: CSP missingconnect-srcdirective — API requests may be blocked.The Content-Security-Policy lacks a
connect-srcdirective. This will default todefault-src 'self', which may block API calls toBASE_URLif it's on a different origin.Add
connect-srcto allow API requests:const imgSrc = `'self' https://storage.googleapis.com data:`; + const connectSrc = `'self' ${BASE_URL}`; res.set({ // ... - 'Content-Security-Policy': `default-src ${defaultSrc}; script-src ${scriptSrc}; style-src ${styleSrc}; frame-ancestors ${frameAncestors}; img-src ${imgSrc}; media-src ${mediaSrc}`, + 'Content-Security-Policy': `default-src ${defaultSrc}; script-src ${scriptSrc}; style-src ${styleSrc}; frame-ancestors ${frameAncestors}; img-src ${imgSrc}; media-src ${mediaSrc}; connect-src ${connectSrc}`,Committable suggestion skipped: line range outside the PR's diff.
wavefront/client/server.cjs-23-36 (1)
23-36: Potential XSS via JSON injection and missing cache headers.Two concerns with this endpoint:
- XSS risk: If any config value contains
</script>, it could break out of the script context. Use proper escaping.- Missing cache headers: Config may change between deployments; add
Cache-Control: no-storeto prevent stale config.app.get('/config.js', (req, res) => { const config = { BASE_URL, APP_ENV, FEATURE_API_SERVICES, }; res.setHeader('Content-Type', 'application/javascript'); res.set({ + 'Cache-Control': 'no-store', 'X-Content-Type-Options': 'nosniff', 'Strict-Transport-Security': 'max-age=63072000; includeSubDomains; preload', }); - res.send(`window.__APP_CONFIG__ = ${JSON.stringify(config)};`); + // Escape </script> to prevent XSS + const safeJson = JSON.stringify(config).replace(/</g, '\\u003c'); + res.send(`window.__APP_CONFIG__ = ${safeJson};`); });wavefront/client/server.cjs-70-77 (1)
70-77: Synchronous file read blocks the event loop on every request.
fs.readFileSyncis called on every request to the catch-all route, which blocks the Node.js event loop and degrades performance under load.Consider caching the file content at startup and only re-reading in development:
+const fs = require('fs'); + +// Cache index.html content at startup +let indexContent; +function getIndexContent() { + if (!indexContent || process.env.NODE_ENV === 'development') { + indexContent = fs.readFileSync(path.join(__dirname, 'dist', 'index.html'), 'utf8'); + } + return indexContent; +} + app.get('*', (req, res) => { // Generate nonce for CSP const nonce = crypto.randomBytes(16).toString('base64'); // ... headers setup ... - // Read the index.html file - const indexPath = path.join(__dirname, 'dist', 'index.html'); - const indexContent = require('fs').readFileSync(indexPath, 'utf8'); + const content = getIndexContent(); // Add nonce to script tags - const modifiedContent = indexContent.replace(/<script/g, `<script nonce="${nonce}"`); + const modifiedContent = content.replace(/<script/g, `<script nonce="${nonce}"`); res.send(modifiedContent); });Committable suggestion skipped: line range outside the PR's diff.
wavefront/client/.env-1-5 (1)
1-5: Add.envto.gitignoreand use.env.exampleinstead.The
.envfile is currently tracked in git, which violates security best practices. Committed.envfiles can expose secrets if added in the future. Recommended approach:
- Create
.env.examplewith placeholder values for this configuration- Add
.envto a.gitignorefile (none currently exists)- Developers can then copy
.env.exampleto.envlocallyAdditionally, verify whether
VITE_APP_ENV=productionis intentional for local development. If you're testing production behavior locally, this is fine; otherwise, consider changing it todevelopmentorlocalto match the actual environment.wavefront/client/src/components/ResourceCard.tsx-38-44 (1)
38-44: MissingstopPropagationon delete button click.The delete button is nested inside the clickable card container. Without
stopPropagation(), clicking the delete button will also trigger the card'sonClickhandler, causing unintended navigation alongside the delete action.<button - onClick={onDeleteClick} + onClick={(e) => { + e.stopPropagation(); + onDeleteClick(e); + }} className="cursor-pointer rounded p-1 text-red-500 opacity-0 transition-opacity hover:bg-red-50 hover:text-red-700 group-hover:opacity-100" title={deleteTitle} >wavefront/client/src/index.css-40-47 (1)
40-47: Fix scrollbar visibility selector.The selector
:hover::-webkit-scrollbar-thumbwon't work as intended. Pseudo-elements like::webkit-scrollbar-thumbcannot have hover states directly. The typical pattern is to apply this on the scrollable container.Consider this approach instead:
::-webkit-scrollbar-thumb { background: rgb(168, 168, 168); - visibility: hidden; border-radius: 16px; } -:hover::-webkit-scrollbar-thumb { - visibility: visible; -}Or if you want to show on container hover:
/* Apply to specific scrollable containers */ .scrollable-container::-webkit-scrollbar-thumb { background: rgb(168, 168, 168); opacity: 0; border-radius: 16px; transition: opacity 0.2s; } .scrollable-container:hover::-webkit-scrollbar-thumb { opacity: 1; }wavefront/client/src/config/env.ts-1-3 (1)
1-3: Add type definitions for runtime configuration.The
window.__APP_CONFIG__global lacks type safety, andgetConfig()returnsany. This defeats TypeScript's type checking and could lead to runtime errors.Add a type declaration:
declare global { interface Window { __APP_CONFIG__?: { BASE_URL?: string; APP_ENV?: string; FEATURE_API_SERVICES?: string; }; } } export const getConfig = (): Window['__APP_CONFIG__'] => { return window.__APP_CONFIG__ || {}; };.github/workflows/build-wavefront-web-develop.yaml-3-18 (1)
3-18: Add an explicitpermissionsblock for least-privilege GITHUB_TOKENThe workflow currently lacks a permissions block, which is a security best practice and flagged by CodeQL. Define minimal permissions required for the workflow to operate.
permissions: contents: read id-token: write # required for google-github-actions/auth if you migrate to OIDCYou can set this either at the workflow root (line 2) or per-job. Currently, the workflow authenticates to GCP using a service account key via
credentials_json; if you transition to OIDC authentication, theid-token: writepermission will be necessary.wavefront/client/src/lib/axios.ts-51-57 (1)
51-57: Handle network errors whereerr.responseis undefined.When a network error occurs (e.g., server unreachable, CORS failure),
err.responseisundefined, causingerrorData.data?.meta?.errorto throw.function (err: IAxiosError) { - const errorData = err.response as IApiResponse; - useNotifyStore.setState({ - visible: true, - type: "error", - message: errorData.data?.meta?.error || "Something went wrong", - }); + const errorData = err.response as IApiResponse | undefined; + const errorMessage = errorData?.data?.meta?.error || err.message || "Something went wrong"; + useNotifyStore.setState({ + visible: true, + type: "error", + message: errorMessage, + });wavefront/client/src/api/voice-agent-service.ts-59-68 (1)
59-68: Add proper typing forinitiateCallresponse.The
initiateCallmethod returnsPromise<any>, which eliminates type safety. Define a proper response type (e.g.,CallInitiationResponse) that describes the structure of the response from the/initiateendpoint.Create a type definition and apply this diff:
+import { + // ... existing imports + CallInitiationResponse, +} from "@app/types/voice-agent"; async initiateCall( agentId: string, data: { to_number: string; from_number?: string } -): Promise<any> { +): Promise<CallInitiationResponse> { - const response = await this.http.post( + const response: IApiResponse</* call data type */> = await this.http.post( `/v1/:appId/floware/v1/voice-agents/${agentId}/initiate`, data ); return response; }Committable suggestion skipped: line range outside the PR's diff.
wavefront/client/src/api/knowledge-base-service.ts-152-184 (1)
152-184: URL query string built via concatenation lacks proper encoding.The
ragQuerymethod builds the URL by string concatenation (lines 162-175), which doesn't URL-encode thequeryparameter. This could cause issues with special characters.Use axios params for proper encoding:
async ragQuery( kbId: string, inferenceId: string, query: string, threshold?: number, topK?: number, vectorWeight?: number, keywordWeight?: number, imageData?: string ): Promise<RagInferenceResponse> { - let url = `/v1/:appId/floware/v1/knowledge-base/${kbId}/augment/${inferenceId}?query=${query}`; - - if (threshold) { - url += `&threshold=${threshold}`; - } - if (topK) { - url += `&top_k=${topK}`; - } - if (vectorWeight) { - url += `&vector_weight=${vectorWeight}`; - } - if (keywordWeight) { - url += `&keyword_weight=${keywordWeight}`; - } + const params: Record<string, string | number> = { query }; + if (threshold) params.threshold = threshold; + if (topK) params.top_k = topK; + if (vectorWeight) params.vector_weight = vectorWeight; + if (keywordWeight) params.keyword_weight = keywordWeight; const data: { image_data?: string } = {}; if (imageData) { data.image_data = imageData; } - const response: RagInferenceResponse = await this.http.post(url, data); + const response: RagInferenceResponse = await this.http.post( + `/v1/:appId/floware/v1/knowledge-base/${kbId}/augment/${inferenceId}`, + data, + { params } + ); return response; }
🟡 Minor comments (18)
wavefront/client/src/components/ui/table.tsx-26-31 (1)
26-31: Fix incorrect Tailwind arbitrary variant syntax.The className
[&>tr]:last:border-b-0mixes arbitrary variant syntax with Tailwind's modifier syntax incorrectly. This will prevent the style from being applied.Apply this diff to correct the CSS selector:
- <tfoot ref={ref} className={cn('bg-muted/50 border-t font-medium [&>tr]:last:border-b-0', className)} {...props} /> + <tfoot ref={ref} className={cn('bg-muted/50 border-t font-medium [&>tr:last-child]:border-b-0', className)} {...props} />wavefront/client/src/assets/icons/rootflo-icon.tsx-12-12 (1)
12-12: Fix the escaped newline characters in the style template.The template literal contains escaped
\ncharacters which appear to be unintentional. These should either be removed or converted to actual newlines for cleaner formatting.Apply this diff to clean up the formatting:
- <style>{`\n .cls-1 {\n fill: ${color ? color : '#231f20'};\n }\n `}</style> + <style>{`.cls-1 { fill: ${color ? color : '#231f20'}; }`}</style>wavefront/client/server.cjs-70-77 (1)
70-77: Missing error handling for missingdist/index.html.If
dist/index.htmldoesn't exist (e.g., before build), the server will crash with an unhandled exception.+ const indexPath = path.join(__dirname, 'dist', 'index.html'); + + if (!require('fs').existsSync(indexPath)) { + return res.status(500).send('Application not built. Run build first.'); + } + - const indexPath = path.join(__dirname, 'dist', 'index.html'); const indexContent = require('fs').readFileSync(indexPath, 'utf8');wavefront/client/src/components/ui/empty.tsx-62-73 (1)
62-73: Type and element mismatch inEmptyDescription.The component is typed with
React.ComponentProps<'p'>but renders a<div>. This could cause issues if consumers pass<p>-specific props or if semantic HTML is important for accessibility.-function EmptyDescription({ className, ...props }: React.ComponentProps<'p'>) { +function EmptyDescription({ className, ...props }: React.ComponentProps<'div'>) { return ( <div data-slot="empty-description"Alternatively, if paragraph semantics are desired, change the element to
<p>:function EmptyDescription({ className, ...props }: React.ComponentProps<'p'>) { return ( - <div + <p data-slot="empty-description" className={cn( 'text-muted-foreground [&>a:hover]:text-primary text-sm/relaxed [&>a]:underline [&>a]:underline-offset-4', className )} {...props} - /> + /> ); }wavefront/client/src/components/ui/separator.tsx-1-1 (1)
1-1: Remove unnecessary 'use client' directive.The
'use client'directive is specific to Next.js App Router, but the PR summary indicates this project uses Vite. While it may be ignored by Vite, it's unnecessary and could cause confusion.-'use client'; - import * as React from 'react';wavefront/client/src/config/env.ts-7-9 (1)
7-9: Feature flag parsing is fragile.The strict string comparison
=== "true"will only match the exact lowercase string "true". Values like"True","TRUE","1", or booleantruewill be treated as disabled.Consider more robust parsing:
const isApiServicesEnabled = - import.meta.env.VITE_FEATURE_API_SERVICES === "true" || - getConfig().FEATURE_API_SERVICES === "true"; + String(import.meta.env.VITE_FEATURE_API_SERVICES).toLowerCase() === "true" || + String(getConfig().FEATURE_API_SERVICES).toLowerCase() === "true";wavefront/client/src/api/user-service.ts-19-25 (1)
19-25: Add return type annotation for consistency.The method is missing a return type annotation, unlike
whoAmI()which declaresPromise<IApiResponse<{ user: IUser }>>. Add the return type for consistency and better type safety.Also, consider whether the
nullbody with email as a query parameter is the intended API contract. If the backend can accept the email in the POST body instead, that would be more conventional:-async resetPasswordEmailSend(email: string) { - return this.http.post("/v1/user/send-reset-password-email", null, { - params: { - email: email, - }, - }); +async resetPasswordEmailSend(email: string): Promise<IApiResponse<unknown>> { + return this.http.post("/v1/user/send-reset-password-email", { email }); }If the API requires query parameters, keep the current pattern and just add the return type.
Committable suggestion skipped: line range outside the PR's diff.
wavefront/client/src/api/tool-service.ts-20-32 (1)
20-32: ChangegetToolNamesAndDetails()return type toToolDetailsResponseThe method calls
/v1/:appId/floware/v1/tools/tool-details, which the backend confirms returnsToolDetailsData, notToolNamesData. Update the method signature and internal type annotation:- async getToolNamesAndDetails(): Promise<ToolNamesResponse> { - const response: IApiResponse<ToolNamesData> = await this.http.get( + async getToolNamesAndDetails(): Promise<ToolDetailsResponse> { + const response: IApiResponse<ToolDetailsData> = await this.http.get( `/v1/:appId/floware/v1/tools/tool-details` ); return response; }The server-side endpoint
/tool-detailsis registered withToolDetailsDataresponse model, notToolNamesData.wavefront/client/src/lib/axios.ts-34-38 (1)
34-38: FragileappIdextraction from URL path.The extraction assumes the URL structure is always
/xxx/{appId}/.... If the URL structure changes or differs across routes, this will silently produce incorrect values or empty strings.Consider a more robust approach or centralized route parameter management.
wavefront/client/src/components/DashboardLayout.tsx-28-39 (1)
28-39: Missingnavigatein useCallback dependency array.The
resetTimercallback usesnavigatebut doesn't include it in the dependency array. This could lead to stale closure issues ifnavigatechanges. Also, the@ts-ignorecan be replaced withwindow.setTimeoutto get the correct browser return type.const resetTimer = useCallback(() => { if (timeoutRef.current) { clearTimeout(timeoutRef.current); } - // @ts-ignore - timeoutRef.current = setTimeout( + timeoutRef.current = window.setTimeout( () => { navigate("/logout"); }, 30 * 60 * 1000 // 30 minutes ); -}, []); +}, [navigate]);wavefront/client/src/components/ui/breadcrumb.tsx-80-80 (1)
80-80: Fix typo in displayName.The displayName is misspelled as "BreadcrumbElipssis" (missing "i"). This will appear incorrectly in React DevTools.
Apply this diff:
-BreadcrumbEllipsis.displayName = 'BreadcrumbElipssis'; +BreadcrumbEllipsis.displayName = 'BreadcrumbEllipsis';wavefront/client/src/components/ui/breadcrumb.tsx-48-59 (1)
48-59: Reconsiderrole="link"on BreadcrumbPage.The
BreadcrumbPagecomponent usesrole="link"on aspanelement representing the current page. According to ARIA practices, the current page in a breadcrumb should not be a link and typically doesn't needrole="link". The combination ofrole="link"witharia-disabled="true"is non-standard.Consider removing
role="link"and relying solely onaria-current="page"to indicate the current location, which is the standard pattern for breadcrumbs.Apply this diff:
<span ref={ref} - role="link" - aria-disabled="true" aria-current="page" className={cn('text-foreground font-normal', className)} {...props} />wavefront/client/src/components/InferencePopup.tsx-421-429 (1)
421-429: Addtype="button"to document remove button.Same issue as the image remove button - this could trigger form submission.
<button + type="button" onClick={() => handleRemoveDocument(index)} className="rounded-full p-1 text-red-500 hover:bg-red-50" title="Remove document" >wavefront/client/src/components/InferencePopup.tsx-346-354 (1)
346-354: Addtype="button"to prevent accidental form submission.The remove image button is inside the form but lacks
type="button". In HTML, buttons default totype="submit", so clicking this could accidentally submit the form.<button + type="button" onClick={handleRemoveImage} className="rounded-full p-1 text-red-500 hover:bg-red-50" title="Remove image" >wavefront/client/src/components/InferencePopup.tsx-498-505 (1)
498-505: Addtype="button"to modal close button.The close button in the modal wrapper should explicitly have
type="button"to prevent form interaction issues.<button + type="button" onClick={() => onClose()} className="rounded-full p-2 text-gray-400 hover:bg-gray-100 hover:text-gray-600" >wavefront/client/src/components/ChatBot.tsx-161-166 (1)
161-166: Alignment logic may not match role-based styling.The alignment in lines 162-165 uses
index % 2 === 0to determineitems-endvsitems-start, but line 157-159 useschat.roleto determinejustify-endvsjustify-start. These could become inconsistent if messages don't strictly alternate between user and assistant.Use role-based alignment consistently:
<div className={clsx( "flex max-w-[70%] flex-col", - index % 2 === 0 ? "items-end" : "items-start" + chat.role === "user" ? "items-end" : "items-start" )} >wavefront/client/src/hooks/data/query-functions.ts-51-73 (1)
51-73: Guard JSON.parse ingetDatasourceQueryFnagainst malformed config payloads
getDatasourceQueryFncallsJSON.parse(responseData.config)on line 65 whenconfigis a string. If the backend sends invalid JSON, this will throw and violate the function's return type contract (Datasource | null), causing the query to error instead of returning null.Wrap the parse in a try/catch and fall back to an empty object:
let config: unknown = responseData.config ?? {}; if (typeof responseData.config === "string") { try { config = JSON.parse(responseData.config); } catch { config = {}; } }Alternatively, if failing hard on malformed config is intentional, add a comment explaining why.
wavefront/client/src/hooks/data/fetch-hooks.ts-339-350 (1)
339-350:useGetWorkflowRuns: backend failures return empty result instead of error stateThe hook correctly wires to
getWorkflowRunsKeyandgetWorkflowRunsQueryFn. However,getWorkflowRunsQueryFnconverts non-success responses into an empty page object, preventing the hook from exposingisError/errorfor backend failures. The consuming component inworkflowPipelineId.tsxexplicitly checks forrunsErrorto display error feedback and a retry button, but this will never trigger since backend errors are swallowed. If the UI needs to distinguish "no runs" from "failed to fetch", change the query function to throw on error meta instead of returning empty data.
| useEffect(() => { | ||
| const token = localStorage.getItem(TOKEN_KEY); | ||
| const currentPath = localStorage.getItem(CURRENT_PATH_KEY); | ||
|
|
||
| if (!token) { | ||
| setAuthenticatedState(false); | ||
| navigate("/login"); | ||
| } else { | ||
| setAuthenticatedState(true); | ||
| } | ||
|
|
||
| if (currentPath) { | ||
| navigate(currentPath); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Fix authentication flow issues.
The current implementation has several problems:
-
Missing dependencies: The useEffect has an empty dependency array but uses
navigateandsetAuthenticatedState. While these are typically stable, it's better to include them or explicitly acknowledge they're stable. -
Navigation logic issue: Lines 23-25 navigate to
currentPathregardless of authentication status. If there's no token, the user is sent to/login(line 18), but then immediately redirected tocurrentPath(line 24), which could cause unexpected behavior. -
No cleanup of currentPath: After navigating to the stored path, it should be removed from localStorage to prevent repeated navigation on subsequent renders.
Apply this diff:
useEffect(() => {
const token = localStorage.getItem(TOKEN_KEY);
const currentPath = localStorage.getItem(CURRENT_PATH_KEY);
if (!token) {
setAuthenticatedState(false);
navigate("/login");
- } else {
+ return;
+ }
+
+ setAuthenticatedState(true);
+
+ if (currentPath) {
+ localStorage.removeItem(CURRENT_PATH_KEY);
navigate(currentPath);
}
-
- if (currentPath) {
- navigate(currentPath);
- }
- }, []);
+ }, [navigate, setAuthenticatedState]);| const getWorkflowRunsQueryFn = async ( | ||
| workflowPipelineId: string, | ||
| offset: number = 0, | ||
| limit: number = 10 | ||
| ): Promise<WorkflowRunListData> => { | ||
| const response = await floConsoleService.workflowService.getWorkflowRuns( | ||
| workflowPipelineId, | ||
| offset, | ||
| limit | ||
| ); | ||
| if (response.data?.meta?.status === "success" && response.data.data) { | ||
| return response.data.data; | ||
| } | ||
| return { | ||
| workflow_runs: [], | ||
| total_count: 0, | ||
| page_size: limit, | ||
| page_number: Math.floor(offset / limit), | ||
| total_pages: 0, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "query-functions.ts" | head -20Repository: rootflo/wavefront
Length of output: 113
🏁 Script executed:
# Read the specific file and location
cat -n wavefront/client/src/hooks/data/query-functions.ts | sed -n '260,300p'Repository: rootflo/wavefront
Length of output: 1427
🏁 Script executed:
# Search for usages of getWorkflowRunsQueryFn
rg "getWorkflowRunsQueryFn" -A 3 -B 3Repository: rootflo/wavefront
Length of output: 2163
🏁 Script executed:
# Look at similar functions in the file to understand the pattern
rg "Promise<.*ListData>" wavefront/client/src/hooks/data/query-functions.ts -B 5 -A 10Repository: rootflo/wavefront
Length of output: 479
🏁 Script executed:
# Check the useQueryInit implementation and error handling
rg "useQueryInit" -A 10 wavefront/client/src/hooks/data/fetch-hooks.ts | head -40Repository: rootflo/wavefront
Length of output: 1347
🏁 Script executed:
# Check if there's error handling or logging in the service layer
rg "workflowService" wavefront/client/src -l | head -5Repository: rootflo/wavefront
Length of output: 409
🏁 Script executed:
# Look at how other similar query functions handle errors
rg "return \[\]" wavefront/client/src/hooks/data/query-functions.ts -B 5Repository: rootflo/wavefront
Length of output: 3629
🏁 Script executed:
# Check how results are consumed in components
rg "useWorkflowRunsQuery" -A 5 -B 2 wavefront/client/srcRepository: rootflo/wavefront
Length of output: 43
🏁 Script executed:
# Search for workflow runs usage in components more broadly
rg "workflow.*run" -i wavefront/client/src/pages -A 2 -B 2 | head -60Repository: rootflo/wavefront
Length of output: 6578
🏁 Script executed:
# Check if there's any global error handling or monitoring
rg "try.*catch" wavefront/client/src/hooks/data/query-functions.tsRepository: rootflo/wavefront
Length of output: 43
🏁 Script executed:
# Look for useQueryInit definition to understand error handling
find . -type f -name "*.ts" -o -name "*.tsx" | xargs rg "export.*useQueryInit" | head -5Repository: rootflo/wavefront
Length of output: 127
🏁 Script executed:
# Read the component file to see full error handling
cat -n wavefront/client/src/pages/apps/[appId]/workflows/pipelines/[workflowPipelineId].tsx | grep -A 30 "runsError" | head -40Repository: rootflo/wavefront
Length of output: 1703
🏁 Script executed:
# Check the useQueryInit implementation
cat -n ./wavefront/client/src/lib/react-query.ts | head -100Repository: rootflo/wavefront
Length of output: 731
getWorkflowRunsQueryFn silently converts backend failures into empty results, preventing error UI from surfacing
When meta.status !== "success" or response.data.data is missing, the function returns an empty WorkflowRunListData instead of throwing. This causes React Query to treat the response as successful (no error state), making the component's error handling at line 432–438 unreachable. Users see "no workflow runs" instead of the "Error loading workflow runs" message when the backend fails or returns unexpected responses.
To surface errors correctly, throw when meta.status !== "success" or the response structure is invalid:
if (response.data?.meta?.status !== "success" || !response.data.data) {
throw new Error(`Failed to fetch workflow runs: ${response.data?.meta?.message || 'Unknown error'}`);
}
return response.data.data;Note: This pattern affects multiple query functions in the file. Consider a consistent error-handling strategy across all query functions.
🤖 Prompt for AI Agents
In wavefront/client/src/hooks/data/query-functions.ts around lines 270 to 290,
the getWorkflowRunsQueryFn currently returns an empty WorkflowRunListData when
response.data?.meta?.status !== "success" or response.data.data is missing;
change it to throw an Error instead so React Query surfaces the failure: check
the response for meta.status === "success" and presence of data, and if not
throw a new Error that includes response.data?.meta?.message or a fallback
message like "Unknown error" (this will allow the consuming component to enter
its error state); apply the same standardized throw-on-invalid-response pattern
to the other query functions in this file for consistency.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wavefront/client/src/components/AppCard.tsx (1)
20-49: Consider minor refinements to event handling and button semantics.Two optional improvements:
Remove unnecessary
preventDefault(line 24): Since the onClick is on adivelement, not a form or link,preventDefault()has no effect and can be removed.Add explicit
type="button"to buttons (lines 29, 39): While not critical, addingtype="button"prevents these buttons from accidentally acting as submit buttons if the component is ever nested in a form.Apply this diff to refine the implementation:
<div className="group relative flex w-full cursor-pointer flex-col gap-8 rounded-xl border border-[#FFF] bg-white/60 p-5" onClick={(e) => { - e.preventDefault(); onClick(app); }} > <div className="absolute right-5 top-5 flex items-center gap-2"> <button + type="button" onClick={(e) => { e.stopPropagation(); handleEditApp(app); }} className="cursor-pointer rounded p-1 text-gray-600 opacity-0 transition-opacity hover:bg-gray-100 hover:text-gray-900 group-hover:opacity-100" title="Edit" > <Pencil className="h-4 w-4" /> </button> <button + type="button" onClick={(e) => { e.stopPropagation(); onDeleteClick(app); }} className="cursor-pointer rounded p-1 text-red-500 opacity-0 transition-opacity hover:bg-red-50 hover:text-red-700 group-hover:opacity-100" title="Delete" > <Trash2 className="h-4 w-4" /> </button> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wavefront/client/src/components/AppCard.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-push-artifact
🔇 Additional comments (3)
wavefront/client/src/components/AppCard.tsx (3)
7-11: LGTM!The interface definition is clear, type-safe, and appropriately structured for the component's requirements.
13-18: LGTM!The navigation setup is clean and follows a clear RESTful pattern for editing resources.
50-58: Implementation is correct.The component properly uses the defined App type fields:
app_name(line 51),public_url(line 53), andupdated_at(line 57), all of which are correctly typed in the App interface.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
wavefront/client/src/api/knowledge-base-service.ts (1)
206-214: Consider defining empty response types for delete operations.While
IApiResponse<any>is common for delete operations, consider defining an explicit empty response type or status response for better type clarity and consistency.+export interface DeleteResponse { + success: boolean; + message?: string; +} + -async deleteSystemPrompt(kbId: string, inferenceId: string): Promise<IApiResponse<any>> { +async deleteSystemPrompt(kbId: string, inferenceId: string): Promise<IApiResponse<DeleteResponse>> { - const response: IApiResponse<any> = await this.http.delete( + const response: IApiResponse<DeleteResponse> = await this.http.delete( `/v1/:appId/floware/v1/knowledge-base/${kbId}/inference/${inferenceId}` ); return response; }Apply similar changes to
deleteKnowledgeBaseanddeleteDocumentmethods.Also applies to: 225-230, 232-240
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wavefront/client/src/api/knowledge-base-service.ts(1 hunks)wavefront/client/src/components/ProtectedLayout.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- wavefront/client/src/components/ProtectedLayout.tsx
🔇 Additional comments (1)
wavefront/client/src/api/knowledge-base-service.ts (1)
96-96: The:appIdplaceholder is properly handled by the axios request interceptor.The axios instance in
wavefront/client/src/lib/axios.ts(lines 33–38) includes logic that replaces the:appIdplaceholder with the actual application ID extracted from the current page URL pathname before sending requests. No changes needed.
| export interface KnowledgeBaseData { | ||
| data: KbData; | ||
| } | ||
|
|
||
| export type KnowledgeBaseDetail = IApiResponse<KbData>; | ||
|
|
||
| // Interface for a single knowledge base response | ||
| export type KnowledgeBaseDetailResponse = IApiResponse<KnowledgeBaseData>; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "knowledge-base-service.ts" -type f | head -5Repository: rootflo/wavefront
Length of output: 113
🏁 Script executed:
# View the type definitions section of the file (lines 1-40)
cat -n wavefront/client/src/api/knowledge-base-service.ts | head -40Repository: rootflo/wavefront
Length of output: 1304
🏁 Script executed:
# Check the actual usage of both types in the service methods
echo "=== KnowledgeBaseDetail usage ==="
rg -nP --type=ts 'KnowledgeBaseDetail[^R]' wavefront/client/src/api/knowledge-base-service.ts -C 2
echo -e "\n=== KnowledgeBaseDetailResponse usage ==="
rg -nP --type=ts 'KnowledgeBaseDetailResponse' wavefront/client/src/api/knowledge-base-service.ts -C 2Repository: rootflo/wavefront
Length of output: 936
🏁 Script executed:
# Check usage in the broader codebase
echo "=== KnowledgeBaseDetail usage in codebase ==="
rg -nP --type=ts 'KnowledgeBaseDetail[^R]' wavefront/client/src --include-dir=. | head -20
echo -e "\n=== KnowledgeBaseDetailResponse usage in codebase ==="
rg -nP --type=ts 'KnowledgeBaseDetailResponse' wavefront/client/src | head -20Repository: rootflo/wavefront
Length of output: 977
🏁 Script executed:
# Check for any comments or documentation about why different types are used
echo "=== Checking for backend API references ==="
grep -r "knowledge-bases" wavefront/client/src --include="*.ts" -B 2 -A 2 | head -40Repository: rootflo/wavefront
Length of output: 3247
🏁 Script executed:
# Find and view IApiResponse definition
rg -nP 'export.*IApiResponse' wavefront/client/src -A 3Repository: rootflo/wavefront
Length of output: 11139
🏁 Script executed:
# View the complete knowledge-base-service.ts to understand method patterns better
wc -l wavefront/client/src/api/knowledge-base-service.tsRepository: rootflo/wavefront
Length of output: 115
🏁 Script executed:
# Get all method signatures and their return types
sed -n '89,248p' wavefront/client/src/api/knowledge-base-service.ts | rg -nP 'async|Promise'Repository: rootflo/wavefront
Length of output: 1030
Consolidate inconsistent response types for knowledge base operations.
KnowledgeBaseDetail (line 24) wraps KbData directly via IApiResponse<KbData>, while KnowledgeBaseDetailResponse (line 27) wraps KnowledgeBaseData, which itself contains data: KbData. This creates unnecessary double-wrapping and inconsistent shapes between getKnowledgeBase() (returns KnowledgeBaseDetail) and createKnowledgeBase() (returns KnowledgeBaseDetailResponse).
Verify that the backend API actually returns different response structures for GET vs. POST, or align these types to use a consistent structure.
🤖 Prompt for AI Agents
In wavefront/client/src/api/knowledge-base-service.ts around lines 20 to 27, the
response types are inconsistent: KnowledgeBaseDetail is IApiResponse<KbData>
while KnowledgeBaseDetailResponse is IApiResponse<KnowledgeBaseData> (which
wraps data: KbData), causing double-wrapping and mismatched shapes between GET
and POST; inspect the actual backend JSON for GET and POST and then choose a
single canonical shape (either IApiResponse<KbData> if the API returns the
object directly, or IApiResponse<KnowledgeBaseData> if it nests under data),
update both type aliases to the chosen shape, update any callers
(getKnowledgeBase/createKnowledgeBase) and tests to match the unified type, and
remove the unused or redundant type alias.
| export interface RagInferenceResultData { | ||
| response: string; | ||
| sources: any[]; // Assuming sources can be any array for now |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any[] with a typed interface for sources.
The sources array is typed as any[], which eliminates type safety for this field. Define a proper interface that describes the source structure returned by RAG queries.
+export interface RagSource {
+ // Define the actual source structure based on your backend response
+ // Example fields:
+ document_id: string;
+ content: string;
+ score?: number;
+}
+
export interface RagInferenceResultData {
response: string;
- sources: any[];
+ sources: RagSource[];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface RagInferenceResultData { | |
| response: string; | |
| sources: any[]; // Assuming sources can be any array for now | |
| export interface RagSource { | |
| // Define the actual source structure based on your backend response | |
| // Example fields: | |
| document_id: string; | |
| content: string; | |
| score?: number; | |
| } | |
| export interface RagInferenceResultData { | |
| response: string; | |
| sources: RagSource[]; | |
| } |
🤖 Prompt for AI Agents
In wavefront/client/src/api/knowledge-base-service.ts around lines 54 to 56,
replace the untyped sources: any[] with a proper exported interface describing
the RAG source structure. Add an exported RagSource (or similar) interface that
includes likely fields such as id (string), title? (string), url? (string),
excerpt? (string), score? (number) and a metadata?: Record<string, unknown> (or
specific typed fields if known), then change RagInferenceResultData.sources to
RagSource[] and update any usages/imports accordingly.
| export interface AllConfigsData { | ||
| id: string; | ||
| llm_model: string; | ||
| display_name: string; | ||
| type: string; | ||
| base_url: string; | ||
| parameters: any; | ||
| is_deleted: boolean; | ||
| created_at: string; | ||
| updated_at: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any with a typed interface for parameters.
The parameters field is typed as any, reducing type safety. Define a proper interface or use a more specific type like Record<string, unknown>.
export interface AllConfigsData {
id: string;
llm_model: string;
display_name: string;
type: string;
base_url: string;
- parameters: any;
+ parameters: Record<string, unknown>;
is_deleted: boolean;
created_at: string;
updated_at: string;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface AllConfigsData { | |
| id: string; | |
| llm_model: string; | |
| display_name: string; | |
| type: string; | |
| base_url: string; | |
| parameters: any; | |
| is_deleted: boolean; | |
| created_at: string; | |
| updated_at: string; | |
| } | |
| export interface AllConfigsData { | |
| id: string; | |
| llm_model: string; | |
| display_name: string; | |
| type: string; | |
| base_url: string; | |
| parameters: Record<string, unknown>; | |
| is_deleted: boolean; | |
| created_at: string; | |
| updated_at: string; | |
| } |
🤖 Prompt for AI Agents
In wavefront/client/src/api/knowledge-base-service.ts around lines 72 to 82, the
AllConfigsData.parameters field is typed as any which reduces type safety;
replace it with a concrete type (either define and export a Parameters interface
that models expected keys or use Record<string, unknown> if structure is
dynamic), update the AllConfigsData.parameters declaration to that type, update
any code that reads parameters to use the typed fields or narrow with type
guards/validation (or cast after validation), and add/adjust unit types or
parsing checks where configs are deserialized to ensure runtime shape matches
the new type.
| async uploadDocument(kbId: string, file: File): Promise<IApiResponse<any>> { | ||
| const formData = new FormData(); | ||
| formData.append("file", file); | ||
|
|
||
| const response: IApiResponse<any> = await this.http.post( | ||
| `/v1/:appId/floware/v1/knowledge-bases/${kbId}/documents`, | ||
| formData, | ||
| { | ||
| headers: { | ||
| "Content-Type": "multipart/form-data", | ||
| }, | ||
| } | ||
| ); | ||
| return response; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Define a proper response type for document upload.
The uploadDocument method returns IApiResponse<any>, which provides no type safety for the upload response. Define an interface that describes the document metadata returned after upload.
+export interface DocumentUploadResponse {
+ id: string;
+ file_name: string;
+ file_type: string;
+ file_size: number;
+ created_at: string;
+}
+
-async uploadDocument(kbId: string, file: File): Promise<IApiResponse<any>> {
+async uploadDocument(kbId: string, file: File): Promise<IApiResponse<DocumentUploadResponse>> {
const formData = new FormData();
formData.append("file", file);
- const response: IApiResponse<any> = await this.http.post(
+ const response: IApiResponse<DocumentUploadResponse> = await this.http.post(
`/v1/:appId/floware/v1/knowledge-bases/${kbId}/documents`,
formData,
{
headers: {
"Content-Type": "multipart/form-data",
},
}
);
return response;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In wavefront/client/src/api/knowledge-base-service.ts around lines 122 to 136,
the uploadDocument method currently returns IApiResponse<any); define a concrete
interface (e.g. IDocumentUploadResponse) that models the returned document
metadata (id, name, size, mimeType, uploadedAt, status, any urls or tags your
API returns) and replace the any with IApiResponse<IDocumentUploadResponse>; add
the interface to this file or a shared types/models file and update
imports/exports accordingly so the method signature enforces the correct shape
for callers.
| if (threshold) params.threshold = threshold; | ||
| if (topK) params.top_k = topK; | ||
| if (vectorWeight) params.vector_weight = vectorWeight; | ||
| if (keywordWeight) params.keyword_weight = keywordWeight; |
There was a problem hiding this comment.
Fix parameter validation to handle valid zero values.
The truthy checks (if (threshold), if (topK), etc.) will exclude valid zero values. Use explicit !== undefined checks instead to allow 0 as a valid parameter.
- if (threshold) params.threshold = threshold;
- if (topK) params.top_k = topK;
- if (vectorWeight) params.vector_weight = vectorWeight;
- if (keywordWeight) params.keyword_weight = keywordWeight;
+ if (threshold !== undefined) params.threshold = threshold;
+ if (topK !== undefined) params.top_k = topK;
+ if (vectorWeight !== undefined) params.vector_weight = vectorWeight;
+ if (keywordWeight !== undefined) params.keyword_weight = keywordWeight;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (threshold) params.threshold = threshold; | |
| if (topK) params.top_k = topK; | |
| if (vectorWeight) params.vector_weight = vectorWeight; | |
| if (keywordWeight) params.keyword_weight = keywordWeight; | |
| if (threshold !== undefined) params.threshold = threshold; | |
| if (topK !== undefined) params.top_k = topK; | |
| if (vectorWeight !== undefined) params.vector_weight = vectorWeight; | |
| if (keywordWeight !== undefined) params.keyword_weight = keywordWeight; |
🤖 Prompt for AI Agents
In wavefront/client/src/api/knowledge-base-service.ts around lines 164 to 167,
the current truthy checks (if (threshold), if (topK), if (vectorWeight), if
(keywordWeight)) incorrectly omit valid zero values; change each conditional to
an explicit undefined check (e.g., if (threshold !== undefined)) so 0 is
accepted, and assign to params as before; apply the same !== undefined pattern
to threshold, topK, vectorWeight, and keywordWeight.
Summary by CodeRabbit
New Features
Infrastructure
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.