[Components] langfuse - new components #15694
Conversation
jcortes
added
action
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
|
Warning Rate limit exceeded@jcortes has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
WalkthroughThis pull request introduces several new modules and updates within the Langfuse component. New action modules are added for adding user feedback and logging traces, while additional source modules facilitate the polling of API events such as new scores and traces. Core changes also include the addition of constants, utilities, and extended API methods within the main application module to support dynamic API interactions, pagination, and error handling. Package dependencies and versioning have been updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant A as AddFeedbackAction
participant L as LangfuseApp
participant API as "/comments API"
U->>A: Trigger add feedback action
A->>L: Call getObjectId(objectType)
L-->>A: Return objectId
A->>API: POST /comments (projectId, objectType, objectId, content)
API-->>A: Return response
A->>U: Return summary message and response
sequenceDiagram
participant P as Polling Module
participant L as LangfuseApp
participant API as Resource API
P->>L: Call paginate/getIterations with parameters
L->>API: Request resources (listScores or listTraces)
API-->>L: Return resource batch
L-->>P: Yield resources
P->>P: Process each resource (generateMeta, processResource)
P->>P: Update lastDate and firstRun state if applicable
Possibly related PRs
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 4
🧹 Nitpick comments (15)
components/langfuse/actions/log-trace/log-trace.mjs (2)
5-64: Ensure robust validation for user-supplied fieldsThe action defines numerous user-facing props (e.g.,
name,input,output,userId,sessionId). Consider adding validation or checks on these fields (e.g., non-empty validations, or data type validations formetadata) to avoid malformed data during ingestion.
73-119: Request error handling for run methodThe
runmethod callsbatchIngestion(...)and directly awaits the response. If an error arises from the API call, the action will fail without a graceful fallback. Integrating a try/catch inruncan improve reliability and user experience by providing more details.components/langfuse/langfuse.app.mjs (3)
8-34: Clarify labeling and usage of projectId vs. Trace IDThe prop labeled
"Trace ID"is stored inprojectId, which might be confusing. Ensure this naming aligns with the actual usage: if it's truly referencing a project or a trace, revise the label or property name for consistency.
51-69: Limit verbose debuggingIn
_makeRequest,debug: trueis set, which can clutter logs when dealing with a large volume of requests. Consider making debug logging optional, or exposing a prop to toggle this behavior.
88-137: Prevent infinite pagination loopIn the
getIterationsmethod, you incrementpageindefinitely until no resources are found or the max is reached. Ensure that the server’s pagination structure is well-defined or provides a final page indicator to avoid edge cases where large page counts could cause excessive loops.components/langfuse/common/utils.mjs (2)
1-7: Improve function naming and error handling.The function name and parameter are too generic. Consider making these changes:
-async function iterate(iterations) { +async function collectAsyncIterableToArray(iterable) { const items = []; + if (!iterable?.[Symbol.asyncIterator] && !iterable?.[Symbol.iterator]) { + throw new TypeError('Input must be iterable or async iterable'); + } for await (const item of iterations) { items.push(item); } return items; }
9-12: Add input validation and array index support.The function needs input validation and could be enhanced to support array indices.
function getNestedProperty(obj, propertyString) { + if (!propertyString) return obj; + if (typeof propertyString !== 'string') { + throw new TypeError('Property path must be a string'); + } const properties = propertyString.split("."); - return properties.reduce((prev, curr) => prev?.[curr], obj); + return properties.reduce((prev, curr) => { + // Support array indices: convert numeric strings to numbers + const key = /^\d+$/.test(curr) ? parseInt(curr, 10) : curr; + return prev?.[key]; + }, obj); }components/langfuse/sources/new-score-received/test-event.mjs (1)
1-22: Document null fields and consider providing realistic test data.Several fields are set to
nullwithout explanation. Consider:
- Adding comments to explain which fields are optional
- Providing realistic test data for required fields
- Using realistic timestamps within the knowledge cutoff date
components/langfuse/common/constants.mjs (1)
1-32: Add JSDoc comments to document constants.Consider adding JSDoc comments to explain the purpose and usage of each constant.
+/** + * Placeholder used in URL templates to be replaced with actual region. + * @type {string} + */ const REGION_PLACEHOLDER = "{region}"; +/** + * Base URL template for Langfuse API. + * @type {string} + */ const BASE_URL = "https://{region}.langfuse.com"; +/** + * API version path. + * @type {string} + */ const VERSION_PATH = "/api/public"; +/** + * Enum for different types of ingestion events. + * @enum {string} + */ const INGESTION_TYPE = { // ... existing properties ... }; +/** + * Key for storing the last processed date. + * @type {string} + */ const LAST_DATE_AT = "lastDateAt"; +/** + * Key for tracking first run state. + * @type {string} + */ const IS_FIRST_RUN = "isFirstRun"; +/** + * Default limit for pagination. + * @type {number} + */ const DEFAULT_LIMIT = 100; +/** + * Maximum number of items to process. + * @type {number} + */ const DEFAULT_MAX = 1000;components/langfuse/sources/new-trace-received/new-trace-received.mjs (1)
9-9: Consider using a more mature version number.Since this is a production component, consider starting with version "1.0.0" instead of "0.0.1" to indicate stability.
components/langfuse/sources/new-score-received/new-score-received.mjs (1)
12-39: Consider adding input validation and error handling.While the implementation looks good, consider adding:
- Validation for the timestamp format in
getLastDateAt()- Error handling for API response parsing in
generateMeta()Example implementation:
getResourcesFnArgs() { return { debug: true, params: { orderBy: "createdAt.desc", - fromTimestamp: this.getLastDateAt(), + fromTimestamp: this.validateTimestamp(this.getLastDateAt()), }, }; }, +validateTimestamp(timestamp) { + if (!timestamp || isNaN(Date.parse(timestamp))) { + throw new Error('Invalid timestamp format'); + } + return timestamp; +}, generateMeta(resource) { + if (!resource?.id || !resource?.name || !resource?.createdAt) { + throw new Error('Invalid resource format'); + } return { id: resource.id, summary: `New Score: ${resource.name}`, ts: Date.parse(resource.createdAt), }; },components/langfuse/actions/add-feedback/add-feedback.mjs (1)
42-63: Enhance error handling in run method.The current implementation could benefit from more specific error handling.
Consider this enhancement:
async run({ $ }) { const { addFeedback, projectId, objectType, objectId, content, } = this; + try { const response = await addFeedback({ $, data: { projectId, objectType, objectId, content, }, }); $.export("$summary", "Successfully added feedback."); return response; + } catch (error) { + if (error.response?.status === 404) { + throw new Error(`Object not found: ${objectId}`); + } + throw error; + } },components/langfuse/sources/common/polling.mjs (2)
27-29: Add JSDoc comments for abstract methods.The unimplemented methods would benefit from JSDoc comments describing their expected behavior and return types.
Example enhancement:
+/** + * Generate metadata for the emitted event + * @param {Object} resource - The resource object + * @returns {Object} Metadata object containing id, summary, and timestamp + * @throws {ConfigurationError} When not implemented by child class + */ generateMeta() { throw new ConfigurationError("generateMeta is not implemented"); }, +/** + * Get the name of the date field used for polling + * @returns {string} The name of the date field + * @throws {ConfigurationError} When not implemented by child class + */ getDateField() { throw new ConfigurationError("getDateField is not implemented"); },Also applies to: 42-53
93-100: Add type checking for the dateField property.The code assumes the dateField exists on the first resource without validation.
Consider this enhancement:
if (resources.length) { const [firstResource] = Array.from(resources); - if (firstResource) { + if (firstResource && dateField in firstResource) { setLastDateAt(firstResource[dateField]); + } else { + throw new Error(`Date field "${dateField}" not found in resource`); } }components/langfuse/package.json (1)
15-17: Consider using a more specific version range.The caret range (
^3.0.3) allows all minor and patch updates. For better stability, consider using a tilde range (~3.0.3) to only allow patch updates."dependencies": { - "@pipedream/platform": "^3.0.3" + "@pipedream/platform": "~3.0.3" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
components/langfuse/actions/add-feedback/add-feedback.mjs(1 hunks)components/langfuse/actions/log-trace/log-trace.mjs(1 hunks)components/langfuse/common/constants.mjs(1 hunks)components/langfuse/common/utils.mjs(1 hunks)components/langfuse/langfuse.app.mjs(1 hunks)components/langfuse/package.json(2 hunks)components/langfuse/sources/common/polling.mjs(1 hunks)components/langfuse/sources/new-score-received/new-score-received.mjs(1 hunks)components/langfuse/sources/new-score-received/test-event.mjs(1 hunks)components/langfuse/sources/new-trace-received/new-trace-received.mjs(1 hunks)components/langfuse/sources/new-trace-received/test-event.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (5)
components/langfuse/actions/log-trace/log-trace.mjs (1)
1-4: Verify dependency usage for UUID importYou're importing
uuidfrom the "uuid" package. Ensure that your project dependencies include"uuid"at an appropriate, stable version, and verify that you have no bundling issues if this is used in a serverless or browser environment.components/langfuse/langfuse.app.mjs (1)
36-40: Validate region in getUrl
this.$auth.regionis embedded directly intoBASE_URL. If an invalid region is passed in from user config, the request could fail. Consider adding a guard or a default fallback forregion.components/langfuse/sources/new-trace-received/test-event.mjs (1)
1-16: Ensure test event consistency with production data schemaThis test event defines
metadataas a string and setspublicto true. Confirm that these fields match real production data to avoid schema mismatches (e.g., ifmetadatais actually expected to be an object).components/langfuse/common/constants.mjs (1)
1-3:❓ Verification inconclusive
Verify the base URL template.
Let's verify that the base URL template is correct and the API endpoint is accessible.
🏁 Script executed:
#!/bin/bash # Description: Check if the base URL template is valid by testing with a sample region # Test EU region curl -I "https://eu.langfuse.com/api/public" # Test US region curl -I "https://us.langfuse.com/api/public"Length of output: 663
Action: Address Unresolved Base URLs
The test using sample regions ("eu" and "us") failed to resolve the host, as indicated by the CURL error (6). This suggests that the base URL template
"https://{region}.langfuse.com"may not point to accessible endpoints for these regions. Please manually verify the DNS configuration and whether the intended region substitutions yield valid URLs. If these endpoints are supposed to be active, consider confirming DNS records or updating the template accordingly.
- In
components/langfuse/common/constants.mjs, ensure that the region placeholder is correctly replaced at runtime.- Re-check the active endpoints for the expected regions.
components/langfuse/sources/new-score-received/new-score-received.mjs (1)
4-11: LGTM! Well-structured component metadata.The component metadata is clear and includes a helpful link to the API documentation.
components/langfuse/sources/new-score-received/new-score-received.mjs
Outdated
Show resolved
Hide resolved
components/langfuse/sources/new-trace-received/new-trace-received.mjs
Outdated
Show resolved
Hide resolved
jcortes
force-pushed
the
langfuse-new-components
branch
from
5ca15aa to
148e84f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/langfuse/actions/add-feedback/add-feedback.mjs (1)
33-37:⚠️ Potential issueAdd content length validation.
The description mentions a 3000 character limit, but there's no validation enforcing this limit.
content: { type: "string", label: "Content", description: "The content of the comment. May include markdown. Currently limited to 3000 characters.", + validate: (value) => { + if (value.length > 3000) { + throw new Error("Content exceeds 3000 characters limit"); + } + }, },
🧹 Nitpick comments (5)
components/langfuse/common/utils.mjs (3)
1-7: Add error handling for iterator errors.The function should handle potential errors during iteration to prevent unhandled promise rejections.
async function iterate(iterations) { const items = []; - for await (const item of iterations) { - items.push(item); - } + try { + for await (const item of iterations) { + items.push(item); + } + } catch (error) { + console.error('Error during iteration:', error); + throw error; + } return items; }
9-12: Add input validation for parameters.The function should validate input parameters to prevent runtime errors.
function getNestedProperty(obj, propertyString) { + if (!obj || typeof propertyString !== 'string') { + return undefined; + } const properties = propertyString.split("."); return properties.reduce((prev, curr) => prev?.[curr], obj); }
14-35: Optimize recursive JSON parsing.The current implementation could be optimized to handle large objects more efficiently.
const parseJson = (input) => { + const cache = new WeakMap(); const parse = (value) => { + if (typeof value === 'object' && value !== null) { + if (cache.has(value)) { + return cache.get(value); + } + } if (typeof(value) === "string") { try { return parseJson(JSON.parse(value)); } catch (e) { return value; } } else if (typeof(value) === "object" && value !== null) { - return Object.entries(value) + const result = Object.entries(value) .reduce((acc, [ key, val, ]) => Object.assign(acc, { [key]: parse(val), }), {}); + cache.set(value, result); + return result; } return value; }; return parse(input); };components/langfuse/langfuse.app.mjs (2)
145-194: Improve logging in pagination logic.Replace console.log statements with proper logging that can be controlled via configuration.
async *getIterations({ resourcesFn, resourcesFnArgs, resourceName, lastDateAt, dateField, max = constants.DEFAULT_MAX, + logger = console.log, }) { let page = 1; let resourcesCount = 0; while (true) { // ... existing code ... if (!nextResources?.length) { - console.log("No more resources found"); + logger("No more resources found"); return; } // ... existing code ... if (resourcesCount >= max) { - console.log("Reached max resources"); + logger("Reached max resources"); return; } // ... existing code ... if (nextResources.length < constants.DEFAULT_LIMIT) { - console.log("No next page found"); + logger("No next page found"); return; } page += 1; } },
131-136: Centralize API version paths in constants.API version paths should be defined in constants to maintain consistency.
// In constants.mjs: +const API_VERSIONS = { + V1: '', + V2: '/v2' +}; // In this file: listPrompts(args = {}) { return this._makeRequest({ - path: "/v2/prompts", + path: `${constants.API_VERSIONS.V2}/prompts`, ...args, }); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
components/langfuse/actions/add-feedback/add-feedback.mjs(1 hunks)components/langfuse/actions/log-trace/log-trace.mjs(1 hunks)components/langfuse/common/constants.mjs(1 hunks)components/langfuse/common/utils.mjs(1 hunks)components/langfuse/langfuse.app.mjs(1 hunks)components/langfuse/package.json(2 hunks)components/langfuse/sources/common/polling.mjs(1 hunks)components/langfuse/sources/new-score-received/new-score-received.mjs(1 hunks)components/langfuse/sources/new-score-received/test-event.mjs(1 hunks)components/langfuse/sources/new-trace-received/new-trace-received.mjs(1 hunks)components/langfuse/sources/new-trace-received/test-event.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- components/langfuse/sources/new-score-received/test-event.mjs
- components/langfuse/sources/new-trace-received/test-event.mjs
- components/langfuse/actions/log-trace/log-trace.mjs
- components/langfuse/sources/common/polling.mjs
- components/langfuse/sources/new-score-received/new-score-received.mjs
- components/langfuse/package.json
- components/langfuse/common/constants.mjs
- components/langfuse/sources/new-trace-received/new-trace-received.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (1)
components/langfuse/actions/add-feedback/add-feedback.mjs (1)
1-9: LGTM! Well-documented module setup.The module metadata is well-structured with clear documentation links and proper versioning.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/aws/actions/s3-upload-file-tmp/s3-upload-file-tmp.mjs (1)
68-86: Consider improving scalability and reliability of folder uploads.While concurrent uploads improve performance, consider these improvements:
- Limit concurrent uploads to prevent memory issues and AWS rate limits
- Add progress tracking for large folders
- Validate S3 key length (max 1024 bytes)
Here's a suggested implementation:
- const files = this.getFilesRecursive(folderPath); - const response = await Promise.all(files.map(async (filePath) => { + const files = await this.getFilesRecursive(folderPath); + const BATCH_SIZE = 10; // Process 10 files at a time + const results = []; + + for (let i = 0; i < files.length; i += BATCH_SIZE) { + const batch = files.slice(i, i + BATCH_SIZE); + const batchResults = await Promise.all(batch.map(async (filePath) => { const fileContent = fs.readFileSync(filePath, { encoding: "base64", }); const relativePath = filePath.substring(folderPath.length + 1); const s3Key = join(prefix, relativePath); + + // Validate S3 key length + if (Buffer.byteLength(s3Key, 'utf8') > 1024) { + throw new Error(`S3 key ${s3Key} exceeds maximum length of 1024 bytes`); + } await uploadFile({ Bucket: bucket, Key: s3Key, Body: Buffer.from(fileContent, "base64"), }); + + $.export("$summary", `Uploaded ${i + batch.indexOf(filePath) + 1}/${files.length} files`); + return { filePath, s3Key, status: "uploaded", }; - })); + })); + results.push(...batchResults); + } + return results;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/aws/actions/s3-upload-file-tmp/s3-upload-file-tmp.mjs(2 hunks)components/aws/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/aws/package.json
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (1)
components/aws/actions/s3-upload-file-tmp/s3-upload-file-tmp.mjs (1)
15-15: LGTM! Version bump is appropriate.The version increment from 1.0.2 to 1.0.3 reflects the addition of new functionality for recursive directory traversal.
components/aws/actions/s3-upload-file-tmp/s3-upload-file-tmp.mjs
Outdated
Show resolved
Hide resolved
jcortes
force-pushed
the
langfuse-new-components
branch
3 times, most recently
from
a7954fb to
ad1d7ab
Compare
|
/approve |
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently, please check "Code review limits" under "Moderation" settings.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/langfuse/langfuse.app.mjs (1)
83-94:⚠️ Potential issueAdd error handling for API requests.
The
_makeRequestmethod should handle API errors gracefully._makeRequest({ $ = this, path, ...args } = {}) { - return axios($, { - ...args, - url: this.getUrl(path), - auth: this.getAuth(), - headers: { - "Content-Type": "application/json", - }, - }); + try { + return axios($, { + ...args, + url: this.getUrl(path), + auth: this.getAuth(), + headers: { + "Content-Type": "application/json", + }, + }); + } catch (error) { + const message = error.response?.data?.message || error.message; + throw new Error(`Langfuse API error: ${message}`); + } },
🧹 Nitpick comments (3)
components/langfuse/sources/new-trace-received/test-event.mjs (1)
1-16: Enhance test event with more realistic data and documentation.While the basic structure is good, consider the following improvements to make the test event more representative of real-world scenarios:
- Add JSDoc comments explaining the purpose and usage of this test event
- Use more realistic test data
- Include additional test tags to cover more scenarios
Here's a suggested improvement:
+/** + * Sample test event for Langfuse trace events. + * Used by new-trace-received.mjs for testing the event emission functionality. + * @example + * import testEvent from './test-event.mjs'; + */ export default { id: "123", - timestamp: "2025-02-20T16:54:05.109Z", + timestamp: new Date().toISOString(), name: "New Trace Received", - input: "2+2", - output: "4", + input: { + prompt: "What is the capital of France?", + temperature: 0.7, + maxTokens: 100 + }, + output: { + response: "The capital of France is Paris.", + tokens: 8, + finishReason: "stop" + }, sessionId: "abc", release: "1.0.0", version: "1.0.0", userId: "123", - metadata: "metadata", + metadata: { + model: "gpt-3.5-turbo", + environment: "production", + clientInfo: { + type: "api", + version: "1.0.0" + } + }, tags: [ - "test1" + "test1", + "llm-response", + "production-ready" ], public: true, };components/langfuse/langfuse.app.mjs (2)
95-144: Add JSDoc documentation for API methods.Consider adding JSDoc documentation to improve code maintainability and IDE support.
+ /** + * Creates a new resource + * @param {Object} args - Request arguments + * @returns {Promise<Object>} API response + */ post(args = {}) { return this._makeRequest({ method: "POST", ...args, }); }, + /** + * Lists all projects + * @param {Object} args - Request arguments + * @returns {Promise<Object>} List of projects + */ listProjects(args = {}) {
145-197: Add parameter validation and documentation for pagination methods.The pagination methods would benefit from parameter validation and clear documentation.
+ /** + * Iterates through paginated resources + * @param {Object} params - Pagination parameters + * @param {Function} params.resourcesFn - Function to fetch resources + * @param {Object} params.resourcesFnArgs - Arguments for the resource function + * @param {string} params.resourceName - Name of the resource in response + * @param {string} [params.lastDateAt] - Filter resources after this date + * @param {string} [params.dateField] - Date field to filter by + * @param {number} [params.max=1000] - Maximum number of resources to return + * @yields {Object} Resource object + */ async *getIterations({ resourcesFn, resourcesFnArgs, resourceName, lastDateAt, dateField, max = constants.DEFAULT_MAX, }) { + if (!resourcesFn || typeof resourcesFn !== 'function') { + throw new Error('resourcesFn must be a function'); + } + if (!resourceName) { + throw new Error('resourceName is required'); + } + let page = 1; let resourcesCount = 0;
🛑 Comments failed to post (2)
components/langfuse/langfuse.app.mjs (2)
9-22:
⚠️ Potential issueFix inconsistent property labeling.
The
projectIdproperty's label and description mention "Trace ID" but the options method lists projects. This mismatch could confuse users.projectId: { type: "string", - label: "Trace ID", - description: "The ID of the trace to attach feedback to or to filter by for events.", + label: "Project ID", + description: "The ID of the project to attach feedback to or to filter by for events.", async options() {📝 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.projectId: { type: "string", label: "Project ID", description: "The ID of the project to attach feedback to or to filter by for events.", async options() { const { data } = await this.listProjects(); return data.map(({ id: value, name: label, }) => ({ label, value, })); }, },
33-64: 🛠️ Refactor suggestion
Add error handling and simplify objectId options.
The
objectIdoptions method could benefit from error handling and simplification.async options({ objectType, page, }) { if (!objectType) { return []; } + const resourceMap = { + [constants.OBJECT_TYPE.TRACE]: this.listTraces, + [constants.OBJECT_TYPE.OBSERVATION]: this.listObservations, + [constants.OBJECT_TYPE.SESSION]: this.listSessions, + [constants.OBJECT_TYPE.PROMPT]: this.listPrompts, + }; - const resourcesFn = { - [constants.OBJECT_TYPE.TRACE]: this.listTraces, - [constants.OBJECT_TYPE.OBSERVATION]: this.listObservations, - [constants.OBJECT_TYPE.SESSION]: this.listSessions, - [constants.OBJECT_TYPE.PROMPT]: this.listPrompts, - }[objectType]; + const resourcesFn = resourceMap[objectType]; if (!resourcesFn) { - return []; + throw new Error(`Unsupported object type: ${objectType}`); } - const { data } = await resourcesFn({ - params: { - page: page + 1, - limit: constants.DEFAULT_LIMIT, - }, - }); + try { + const { data } = await resourcesFn({ + params: { + page: page + 1, + limit: constants.DEFAULT_LIMIT, + }, + }); - return data.map(({ - id: value, name: label, - }) => ({ - label, - value: value || label, - })); + return data.map(({ + id: value, name: label, + }) => ({ + label: label || value, + value, + })); + } catch (error) { + console.error(`Failed to fetch ${objectType} options:`, error); + throw new Error(`Failed to fetch ${objectType} options: ${error.message}`); + } },📝 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.async options({ objectType, page, }) { if (!objectType) { return []; } const resourceMap = { [constants.OBJECT_TYPE.TRACE]: this.listTraces, [constants.OBJECT_TYPE.OBSERVATION]: this.listObservations, [constants.OBJECT_TYPE.SESSION]: this.listSessions, [constants.OBJECT_TYPE.PROMPT]: this.listPrompts, }; const resourcesFn = resourceMap[objectType]; if (!resourcesFn) { throw new Error(`Unsupported object type: ${objectType}`); } try { const { data } = await resourcesFn({ params: { page: page + 1, limit: constants.DEFAULT_LIMIT, }, }); return data.map(({ id: value, name: label, }) => ({ label: label || value, value, })); } catch (error) { console.error(`Failed to fetch ${objectType} options:`, error); throw new Error(`Failed to fetch ${objectType} options: ${error.message}`); } },
jcortes
force-pushed
the
langfuse-new-components
branch
from
ad1d7ab to
1614f4c
Compare
|
/approve |
WHY
Resolves #15630
Summary by CodeRabbit
New Features
Chores