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

Add pre commit #22

Merged
merged 2 commits into from
Oct 27, 2023
Merged

Add pre commit #22

merged 2 commits into from
Oct 27, 2023

Conversation

nicolewhite
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Oct 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
autoblocks-examples ✅ Ready (Inspect) Visit Preview Oct 27, 2023 0:54am

@nicolewhite nicolewhite merged commit 51b97b8 into main Oct 27, 2023
@nicolewhite nicolewhite deleted the add-pre-commit branch October 27, 2023 00:57
@review-agent-prime
Copy link

The changes in the PR look good overall. However, I have a few suggestions to improve the code:

  1. In the pre-commit job, you are setting up Python but not specifying a Python version. It's a good practice to specify the Python version to ensure the workflow is consistent across different runs. You can do this by adding a with clause to the actions/setup-python@v4 step:
- uses: actions/setup-python@v4
  with:
    python-version: '3.x' # or any specific version you want to use
  1. The pre-commit/[email protected] action is deprecated and no longer maintained. It's recommended to use pre-commit/[email protected] instead:
- uses: pre-commit/[email protected]
  1. In the test-readmes-up-to-date job, you are using actions/checkout@v4 which is not the latest version. It's recommended to use the latest version actions/checkout@v3:
- name: Checkout repository
  uses: actions/checkout@v3

Please consider these suggestions to improve the workflow.
The pull request looks good overall, but I have a few suggestions to improve it:

  1. In the .pre-commit-config.yaml file, you have specified the language_version for the black hook as python3. It would be better to specify the exact Python version you are using, for example python3.8, to ensure consistent formatting across different environments.

  2. In the .prettierrc.yaml file, you have set tabWidth to 2. While this is largely a matter of personal preference, a tabWidth of 4 is more common in many coding standards, including PEP 8 for Python. Consider changing this to ensure consistency with other developers.

Here are the suggested changes:

## .pre-commit-config.yaml

@@ -0,0 +1,18 @@
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.5.0
+    hooks:
+      - id: trailing-whitespace
+      - id: end-of-file-fixer
+      - id: check-yaml
+
+  - repo: https://github.com/psf/black
+    rev: '23.10.1'
+    hooks:
+      - id: black
+        language_version: python3.8
+
+  - repo: https://github.com/pre-commit/mirrors-prettier
+    rev: v3.0.3
+    hooks:
+      - id: prettier

## .prettierrc.yaml

@@ -0,0 +1,4 @@
+trailingComma: all
+tabWidth: 4
+semi: true
+singleQuote: true

Please consider these suggestions and update the PR accordingly.
The changes in this PR are mostly related to documentation and do not involve any code changes. The changes are consistent across multiple README files and involve the addition of a new line and the removal of a line indicating the .env file.

However, the removal of the .env line might cause confusion for users who are not familiar with the concept of environment variables. It might be better to keep this line and add a brief explanation about what the .env file is and why it's needed.

Here's a suggestion for the change:

- Create a file named `.env` in this folder and include the following environment variables:
-`.env`
+ Create a file named `.env` in this folder. This file will store your environment variables, which are used to store sensitive information like API keys. Include the following environment variables in the `.env` file:

This change should be applied to all the README files where this line was removed.
The changes in this PR are mostly about adding semicolons at the end of the statements and changing double quotes to single quotes in import statements. These changes are good for maintaining code consistency and following JavaScript best practices.

However, I noticed that in JavaScript/langchain/src/index.js, the DynamicTool object is being created without any properties. This might cause issues if the DynamicTool class expects certain properties during instantiation. Please ensure that the necessary properties are provided.

Here's a suggestion:

new DynamicTool({
  name: "Today's Date",
  description: "This tool provides the current date",
});

Please replace the description with the actual description of the tool.
The changes in this PR are related to SVG files. It seems like the SVG files have been updated, but the changes are not clear as the SVG paths are not human-readable.

Please ensure that the updated SVGs are displaying as expected in the application. If there are any visual changes, it would be helpful to include screenshots in the PR to make the changes clear to reviewers.

Also, it's a good practice to add a newline at the end of the file. This is a common convention that keeps the last line of code safe from certain types of file processing utilities that might not fully process the last line of the file if it doesn't end with a newline.

Here is a suggestion for the updated SVG files:

<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 283 64"><path fill="black" d="M141 16c-11 0-19 7-19 18s9 18 20 18c7 0 13-3 16-7l-7-5c-2 3-6 4-9 4-5 0-9-3-10-7h28v-3c0-11-8-18-19-18zm-9 15c1-4 4-7 9-7s8 3 9 7h-18zm117-15c-11 0-19 7-19 18s9 18 20 18c6 0 12-3 16-7l-8-5c-2 3-5 4-8 4-5 0-9-3-11-7h28l1-3c0-11-8-18-19-18zm-10 15c2-4 5-7 10-7s8 3 9 7h-19zm-39 3c0 6 4 10 10 10 4 0 7-2 9-5l8 5c-3 5-9 8-17 8-11 0-19-7-19-18s8-18 19-18c8 0 14 3 17 8l-8 5c-2-3-5-5-9-5-6 0-10 4-10 10zm83-29v46h-9V5h9zM37 0l37 64H0L37 0zm92 5-27 48L74 5h10l18 30 17-30h10zm59 12v10l-3-1c-6 0-10 4-10 10v15h-9V17h9v9c0-5 6-9 13-9z"/></svg>
<svg xmlns="http://www.w3.org/2000/svg" width="40" height="31" fill="none"><g opacity=".9"><path fill="url(#a)" d="M13 .4v29.3H7V6.3h-.2L0 10.5V5L7.2.4H13Z"/><path fill="url(#b)" d="M28.8 30.1c-2.2 0-4-.3-5.7-1-1.7-.8-3-1.8-4-3.1a7.7 7.7 0 0 1-1.4-4.6h6.2c0 .8.3 1.4.7 2 .4.5 1 .9 1.7 1.2.7.3 1.6.4 2.5.4 1 0 1.7-.2 2.5-.5.7-.3 1.3-.8 1.7-1.4.4-.6.6-1.2.6-2s-.2-1.5-.7-2.1c-.4-.6-1-1-1.8-1.4-.8-.4-1.8-.5-2.9-.5h-2.7v-4.6h2.7a6 6 0 0 0 2.5-.5 4 4 0 0 0 1.7-1.3c.4-.6.6-1.3.6-2a3.5 3.5 0 0 0-2-3.3 5.6 5.6 0 0 0-4.5 0 4 4 0 0 0-1.7 1.2c-.4.6-.6 1.2-.6 2h-6c0-1.7.6-3.2 1.5-4.5 1-1.3 2.2-2.3 3.8-3C25 .4 26.8 0 28.8 0s3.8.4 5.3 1.1c1.5.7 2.7 1.7 3.6 3a7.2 7.2 0 0 1 1.2 4.2c0 1.6-.5 3-1.5 4a7 7 0 0 1-4 2.2v.2c2.2.3 3.8 1 5 2.2a6.4 6.4 0 0 1 1.6 4.6c0 1.7-.5 3.1-1.4 4.4a9.7 9.7 0 0 1-4 3.1c-1.7.8-3.7 1.1-5.8 1.1Z"/></g><defs><linearGradient id="a" x1="20" x2="20" y1="0" y2="30.1" gradientUnits="userSpaceOnUse"><stop/><stop offset="1" stop-color="#3D3D3D"/></linearGradient><linearGradient id="b" x1="20" x2="20" y1="0" y2="30.1" gradientUnits="userSpaceOnUse"><stop/><stop offset="1" stop-color="#3D3D3D"/></linearGradient></defs></svg>
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 394 80"><path fill="#000" d="M262 0h68.5v12.7h-27.2v66.6h-13.6V12.7H262V0ZM149 0v12.7H94v20.4h44.3v12.6H94v21h55v12.6H80.5V0h68.7zm34.3 0h-17.8l63.8 79.4h17.9l-32-39.7 32-39.6h-17.9l-23 28.6-23-28.6zm18.3 56.7-9-11-27.1 33.7h17.8l18.3-22.7z"/><path fill="#000" d="M81 79.3 17 0H0v79.3h13.6V17l50.2 62.3H81Zm252.6-.4c-1 0-1.8-.4-2.5-1s-1.1-1.6-1.1-2.6.3-1.8 1-2.5 1.6-1 2.6-1 1.8.3 2.5 1a3.4 3.4 0 0 1 .6 4.3 3.7 3.7 0 0 1-3 1.8zm23.2-33.5h6v23.3c0 2.1-.4 4-1.3 5.5a9.1 9.1 0 0 1-3.8 3.5c-1.6.
The SVG file seems to be identical to the previous version, except for the missing newline at the end of the file. It's a good practice to always end a file with a newline. This is because some tools like `cat`, `awk`, `sed`, etc. might not process the last line correctly if it doesn't end with a newline.

Here's the suggested change:

```suggestion
+<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 394 80"><path fill="#000" d="M262 0h68.5v12.7h-27.2v66.6h-13.6V12.7H262V0ZM149 0v12.7H94v20.4h44.3v12.6H94v21h55v12.6H80.5V0h68.7zm34.3 0h-17.8l63.8 79.4h17.9l-32-39.7 32-39.6h-17.9l-23 28.6-23-28.6zm18.3 56.7-9-11-27.1 33.7h17.8l18.3-22.7z"/><path fill="#000" d="M81 79.3 17 0H0v79.3h13.6V17l50.2 62.3H81Zm252.6-.4c-1 0-1.8-.4-2.5-1s-1.1-1.6-1.1-2.6.3-1.8 1-2.5 1.6-1 2.6-1 1.8.3 2.5 1a3.4 3.4 0 0 1 .6 4.3 3.7 3.7 0 0 1-3 1.8zm23.2-33.5h6v23.3c0 2.1-.4 4-1.3 5.5a9.1 9.1 0 0 1-3.8 3.5c-1.6.8-3.5 1.3-5.7 1.3-2 0-3.7-.4-5.3-1s-2.8-1.8-3.7-3.2c-.9-1.3-1.4-3-1.4-5h6c.1.8.3 1.6.7 2.2s1 1.2 1.6 1.5c.7.4 1.5.5 2.4.5 1 0 1.8-.2 2.4-.6a4 4 0 0 0 1.6-1.8c.3-.8.5-1.8.5-3V45.5zm30.9 9.1a4.4 4.4 0 0 0-2-3.3 7.5 7.5 0 0 0-4.3-1.1c-1.3 0-2.4.2-3.3.5-.9.4-1.6 1-2 1.6a3.5 3.5 0 0 0-.3 4c.3.5.7.9 1.3 1.2l1.8 1 2 .5 3.2.8c1.3.3 2.5.7 3.7 1.2a13 13 0 0 1 3.2 1.8 8.1 8.1 0 0 1 3 6.5c0 2-.5 3.7-1.5 5.1a10 10 0 0 1-4.4 3.5c-1.8.8-4.1 1.2-6.8 1.2-2.6 0-4.9-.4-6.8-1.2-2-.8-3.4-2-4.5-3.5a10 10 0 0 1-1.7-5.6h6a5 5 0 0 0 3.5 4.6c1 .4 2.2.6 3.4.6 1.3 0 2.5-.2 3.5-.6 1-.4 1.8-1 2.4-1.7a4 4 0 0 0 .8-2.4c0-.9-.2-1.6-.7-2.2a11 11 0 0 0-2.1-1.4l-3.2-1-3.8-1c-2.8-.7-5-1.7-6.6-3.2a7.2 7.2 0 0 1-2.4-5.7 8 8 0 0 1 1.7-5 10 10 0 0 1 4.3-3.5c2-.8 4-1.2 6.4-1.2 2.3 0 4.4.4 6.2 1.2 1.8.8 3.2 2 4.3 3.4 1 1.4 1.5 3 1.5 5h-5.8z"/></svg>\n

This adds a newline at the end of the file.
The changes in this PR are mostly about adding semicolons at the end of the import statements and after the return statements in the functions. This is a good practice in JavaScript to avoid automatic semicolon insertion (ASI) issues.

In the index.tsx file, a trailing comma is added in the ternary operator inside the className attribute. This is also a good practice as it makes future changes in the code easier to manage and reduces the chance of errors.

However, I noticed that the showAvatar variable is used in a ternary operator but it's not defined anywhere in the provided code. Please make sure to define it before using it.

const showAvatar = /* your condition */;

Also, it's a good practice to use meaningful names for boolean variables. If showAvatar is a boolean, consider renaming it to isAvatarVisible or something similar to make the code more readable.

const isAvatarVisible = /* your condition */;

Overall, the changes in this PR look good. However, there are a few minor improvements that could be made:

  1. In JavaScript/chatbot-nextjs/src/pages/api/chat.ts, you are parsing the request body without checking if it exists or not. This could potentially throw an error if the request body is undefined. Consider adding a check before parsing the request body.
const requestBody = req.body ? JSON.parse(req.body) : {};
const { userInput, pastMessages, traceId, autoblocksIngestionKey } = requestBody;
  1. In JavaScript/novel-ai-text-editor/src/app/api/generate/route.ts, you are directly accessing the OPENAI_API_KEY from process.env. It's a good practice to use a configuration file or environment variable handler to manage environment variables. This way, you can handle missing environment variables in a single place and keep the code DRY.

  2. In JavaScript/novel-ai-text-editor/tailwind.config.ts, you have added semicolons at the end of import and export statements. While this is not an issue, it's important to maintain consistency in code style throughout the project. If the project doesn't use semicolons elsewhere, consider removing them here for consistency.
    The changes in this PR are mostly about code formatting and style, which are generally good. However, there are a few points that could be improved:

  3. In both main.py files, the openai.ChatCompletion.create method is called without any error handling. It would be better to wrap this call in a try-except block to handle potential exceptions that might be thrown. This way, the program won't crash unexpectedly and can provide a more informative error message.

try:
    openai_response = openai.ChatCompletion.create(**request_params)
except Exception as e:
    print(f"Error occurred while calling OpenAI: {e}")
    tracer.send_event(
        "ai.error",
        properties=dict(
            error=dict(
                type=type(e).__name__,
                message=str(e),
                stacktrace=traceback.format_exc(),
            ),
        ),
    )
  1. In the main.py file under openai-manual, the request_params dictionary is defined twice. This seems to be a mistake and the second definition should be removed.
# Remove this duplicate definition
request_params = dict(
    model="gpt-3.5-turbo",
    messages=messages,
    temperature=0.7,
    ...
)
  1. The tracer.send_event method is called with the same arguments in multiple places. To reduce code duplication, you could create a helper function that takes the event type and properties as arguments and calls tracer.send_event.
def send_tracer_event(event_type, properties):
    tracer.send_event(event_type, properties=properties)

Then you can replace the calls to tracer.send_event with calls to this new helper function.

send_tracer_event("ai.request", request_params)
...
send_tracer_event("ai.response", dict(response=openai_response, latency=(time.time() - start_time) * 1000))
...
send_tracer_event("ai.error", dict(error=dict(type=type(error).__name__, message=str(error), stacktrace=traceback.format_exc())))

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