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

refactor: abstracting the creation of a vector store #47

Merged
merged 6 commits into from
Aug 2, 2023

Conversation

SEBRATHEZEBRA
Copy link
Contributor

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 31, 2023

LOGAF Level 2 - /home/runner/work/code-review-gpt/code-review-gpt/src/common/model/AIModel.ts

The API key is exposed in the code. Consider using environment variables to store sensitive information like API keys. For example, you can use process.env.OPENAI_API_KEY instead of passing the key directly to the AIModel constructor.


LOGAF Level 3 - /home/runner/work/code-review-gpt/code-review-gpt/src/test/run/runTest.ts

The function runTest does not handle errors that may occur during the askAI and vectorStore.similaritySearchWithScore operations. Consider adding error handling to ensure that the function behaves correctly even when errors occur.

Example:

try {
  const { markdownReport: reviewResponse } = await askAI(prompts, modelName, false);
  // rest of the code
} catch (error) {
  console.error(`Failed to call AI model: ${modelName}`, error);
}

LOGAF Level 2 - /home/runner/work/code-review-gpt/code-review-gpt/src/review/prompt/makeSlimmedFile.ts

  1. The function makeSlimmedFile is quite long and does a lot of things. Consider breaking it down into smaller, more manageable functions. This will make the code easier to read and maintain.

  2. The error messages are quite verbose and could be simplified. Instead of logging the entire message, consider logging just the key information and then throwing an error with the full details. This will make the logs easier to read and understand.

  3. The function makeSlimmedFile does not have any comments. Consider adding comments to explain what the function does and how it works. This will make the code easier to understand for other developers.

Example:

/**
 * This function does X, Y, and Z.
 *
 * @param {type} paramName - description
 * @returns {type} description
 */
function functionName(paramName) {
  // code
}

🔑💣🔍


Powered by Code Review GPT

@github-actions
Copy link
Contributor

github-actions bot commented Jul 31, 2023

Test results summary:

✅ [PASS] - Test case: Bad variable name
✅ [PASS] - Test case: Exposed secret
⚠️ [WARN] - Test case: Too many nested loops
✅ [PASS] - Test case: Unawaited Promise

SUMMARY: ✅ PASS: 3 - ⚠️ WARN: 1 - ❌ FAIL: 0


Tests Powered by Code Review GPT

import { OpenAIEmbeddings } from 'langchain/embeddings/openai';

describe('CreateMemoryStore function', () => {
const initialFiles = [
Copy link
Owner

Choose a reason for hiding this comment

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

can you make this file in the testFiles and then read it here. Makes the code a lot nicer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done :)

new OpenAIEmbeddings(),
{}
);
expect(result).toEqual(expectedResult);
Copy link
Owner

Choose a reason for hiding this comment

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

also can you check that a simularitySearchWithScore returns a number. Thats the most used method on this class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done :)

@SEBRATHEZEBRA SEBRATHEZEBRA merged commit a3e4fa5 into main Aug 2, 2023
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.

[NPM packaging] AADev I have abstracted creating a MemoryVectorStore
2 participants