-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[Audit] lamini improvements #16877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Audit] lamini improvements #16877
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughSeveral Lamini action components were updated, mainly with version increments and minor property description changes. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateFineTuneJobAction
participant LaminiAPI
User->>CreateFineTuneJobAction: Trigger action (waitForCompletion = true)
CreateFineTuneJobAction->>LaminiAPI: Create fine-tune job
LaminiAPI-->>CreateFineTuneJobAction: Return job ID
alt waitForCompletion = true
loop Until job completes or max retries
CreateFineTuneJobAction->>LaminiAPI: Poll job status
LaminiAPI-->>CreateFineTuneJobAction: Return job status
alt Job completed
CreateFineTuneJobAction-->>User: Return final job status
else Job failed
CreateFineTuneJobAction-->>User: Throw error
else Still running
CreateFineTuneJobAction->>CreateFineTuneJobAction: Wait and rerun
end
end
else waitForCompletion = false
CreateFineTuneJobAction-->>User: Return job creation response
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/lamini/actions/evaluate-job/evaluate-job.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/lamini/actions/create-fine-tune-job/create-fine-tune-job.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/lamini/lamini.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/lamini/actions/create-fine-tune-job/create-fine-tune-job.mjs (1)
82-83: Consider longer timeout or configurable delays for fine-tuning jobs.The current maximum wait time is 7.5 minutes (15 retries × 30 seconds). Fine-tuning jobs can potentially take longer depending on dataset size and model complexity. Consider:
- Increasing MAX_RETRIES
- Making the delay configurable as a prop
- Implementing exponential backoff to reduce API calls while still checking frequently at the start
Also applies to: 129-131
📜 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 (8)
components/lamini/actions/create-fine-tune-job/create-fine-tune-job.mjs(3 hunks)components/lamini/actions/evaluate-job/evaluate-job.mjs(1 hunks)components/lamini/actions/generate-completion/generate-completion.mjs(1 hunks)components/lamini/actions/get-batch-completions/get-batch-completions.mjs(1 hunks)components/lamini/actions/upload-dataset/upload-dataset.mjs(2 hunks)components/lamini/lamini.app.mjs(1 hunks)components/lamini/package.json(1 hunks)components/lamini/sources/get-job-status/get-job-status.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (8)
components/lamini/package.json (1)
3-3: LGTM! Version bump aligns with semantic versioning.The minor version increment from 0.1.0 to 0.1.1 is appropriate for the backwards-compatible enhancements being introduced in this PR.
components/lamini/actions/generate-completion/generate-completion.mjs (1)
8-8: LGTM! Coordinated version bump.The patch version increment is appropriate and aligns with the coordinated version updates across the Lamini action suite.
components/lamini/actions/get-batch-completions/get-batch-completions.mjs (1)
8-8: LGTM! Consistent version increment.The patch version bump maintains consistency with other actions in the Lamini component suite.
components/lamini/lamini.app.mjs (1)
41-44:✅ Verification successful
Verify that dependent components handle the API change appropriately.
The
jobIdoptions method has undergone a significant API change:
- Parameter change:
includeFineTunedModels→filterfunction- Behavioral change: Default now shows all jobs instead of only completed jobs
- Breaking change: Existing consumers expecting only completed jobs may be affected
While this change aligns with the PR objectives for more flexible job handling, ensure that all dependent actions (like
evaluate-job) explicitly provide appropriate filters to maintain expected behavior.Let me verify that dependent components are properly updated to handle this change:
🏁 Script executed:
#!/bin/bash # Description: Check for any usage of the old includeFineTunedModels parameter and verify filter usage # Expected: No remaining usage of old parameter, proper filter functions in place # Search for any remaining usage of includeFineTunedModels echo "=== Searching for old includeFineTunedModels parameter usage ===" rg "includeFineTunedModels" --type js # Search for jobId propDefinition usage to see how filters are applied echo "=== Searching for jobId propDefinition usage ===" rg -A 10 -B 2 "jobId.*propDefinition" --type js # Search for evaluate-job specifically mentioned in AI summary echo "=== Checking evaluate-job implementation ===" fd "evaluate-job" --exec cat {}Length of output: 1494
🏁 Script executed:
#!/bin/bash # List all JS files in the lamini/actions folder that reference 'jobId' echo "=== Files in components/lamini/actions referencing 'jobId' ===" rg -l "jobId" --type js components/lamini/actions # Show surrounding context for each occurrence to verify propDefinition usage echo "=== Context for 'jobId' usages in actions ===" rg -n "jobId" -C5 --type js components/lamini/actionsLength of output: 351
🏁 Script executed:
#!/bin/bash # List all JavaScript and MJS files under components/lamini/actions echo "=== JS/MJS files in components/lamini/actions ===" fd -e js -e mjs components/lamini/actions # Search for all 'jobId' references with context to confirm propDefinition overrides echo "=== Context for 'jobId' usages in actions ===" rg -n "jobId" -C5 components/lamini/actionsLength of output: 8536
Dependent components correctly preserve previous behavior
No instances of the old
includeFineTunedModelsparameter remain, and the only consumer of thejobIdoptions method—components/lamini/actions/evaluate-job/evaluate-job.mjs—explicitly supplies the original completed‐jobs filter:
- In
evaluate-job.mjs(lines 11–17), thepropDefinitionpasses
filter: ({ status }) => status === "COMPLETED",
ensuring only completed jobs appear by default.Since all dependent actions have been updated to provide their own filter, there are no breaking changes to address.
components/lamini/actions/upload-dataset/upload-dataset.mjs (1)
10-10: LGTM! Version bump and clarified descriptions.The version increment and simplified property descriptions are appropriate. The removal of CSV references from the descriptions better aligns with the actual implementation, which only supports JSONL/JSONLINES formats.
Also applies to: 22-22, 27-27
components/lamini/sources/get-job-status/get-job-status.mjs (1)
11-11: LGTM! Consistent version bump.The version increment aligns with the coordinated package update.
components/lamini/actions/evaluate-job/evaluate-job.mjs (1)
7-7: Good addition of job status filtering.The version increment and the filter to show only completed jobs for evaluation make perfect sense. This improves the user experience by preventing selection of jobs that cannot be evaluated.
Also applies to: 15-17
components/lamini/actions/create-fine-tune-job/create-fine-tune-job.mjs (1)
9-9: Well-implemented polling logic for job completion.The addition of
waitForCompletionoption and the associated polling implementation using$.flow.rerunis clean and follows Pipedream patterns correctly. The state management between runs and error handling are properly implemented.Also applies to: 52-58, 82-161
|
Hi everyone, all test cases are passed! Ready for release! Test report |
WHY
Users can now choose to either get an immediate response after job creation or wait for the job to complete, making the component more flexible for different use cases.
Summary by CodeRabbit
New Features
Improvements
Chores