Skip to content
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

Support for direct call of genaiscript from js #925

Merged
merged 14 commits into from
Dec 9, 2024
Merged

Support for direct call of genaiscript from js #925

merged 14 commits into from
Dec 9, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Dec 9, 2024

This is more a v0 to get things going.

  • Exported "runScript' function exposes the same interfaces as the CLI.
  • Uses a worker to avoid polluting the main context with globas

Typings coming up later.


The main changes introduced by this GIT_DIFF are as follows:

  • Worker Threads Implementation: The addition of worker threads allows for offloading computationally intensive tasks, such as script execution, to separate threads. This improves the responsiveness and scalability of the application.

  • Refactoring of Script Execution Logic: The script execution logic from "run.ts" has been moved to "worker.ts". This refactoring enhances modularity, making it easier to manage and extend individual components.

  • API Changes: While not directly visible in this diff, the changes imply modifications to public APIs in files like prompt_template.d.ts and prompt_type.ts. These modifications ensure that users can interact with the updated script execution features seamlessly.

  • Code Duplication Reduction: The removal of duplicate code between "run.ts" and "worker.ts" through refactoring improves maintainability and reduces potential for errors.

  • Performance Enhancements: By offloading tasks to worker threads, applications utilizing this CLI tool can handle more complex scripts concurrently without significant performance degradation.

generated by pr-describe

Copy link

github-actions bot commented Dec 9, 2024

LGTM 🚀

The changes in the PR look good. The introduction of worker threads can help improve performance by offloading heavy tasks to separate threads, reducing the load on the main thread and making the application more scalable.

Summary of Changes:

  1. Introduction of Worker Threads:

    • Added new files worker.ts, api.ts, and moved some logic into these files.
    • Updated existing files like run.ts, server.ts, and main entry file to utilize worker threads for running scripts in separate threads.
  2. Refactoring:

    • Renamed runScript to runScriptInternal in run.ts.
    • Moved script execution logic from run.ts to api.ts.

Functional Concerns:

  • The PR introduces significant changes that could impact the application's behavior, especially if not thoroughly tested. Ensure thorough regression testing is done.
  • Consider adding unit and integration tests for the worker threads to verify their functionality under various conditions.

Additional Recommendations:

  • Load Testing:

    • Perform load testing to ensure the application maintains performance and does not experience bottlenecks.
  • Documentation:

    • Update documentation to reflect the changes, especially how the new worker thread architecture works.

Overall, the PR looks good with a high potential to improve the scalability and performance of the application. Ensure comprehensive testing to catch any unforeseen issues.

generated by pr-review

import { runScript } from "genaiscript/api"

const { exitCode, results } = await runScript("summarize", ["myfile.txt"])
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Front matter is missing at the beginning of the file. It should include a title and sidebar order.

generated by pr-docs-review-commit missing_frontmatter

## Using a the CLI as a Node.JS API

The CLI can be imported and [used as an API in your Node.JS application](/genaiscript/reference/cli/api).

## Topics

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The link to the API documentation should be more descriptive than "Using a the CLI as a Node.JS API". Consider using a more specific title.

generated by pr-docs-review-commit link_to_api_doc

title: Node.JS API
sidebar:
order: 50
---
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Front matter is missing at the beginning of the file. It should include a title and sidebar order.

generated by pr-docs-review-commit missing_frontmatter

@pelikhan pelikhan merged commit 8c0b18f into main Dec 9, 2024
11 checks passed
@pelikhan pelikhan deleted the api branch December 9, 2024 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant