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

fix: returnDataToKernel #44

Open
wants to merge 14 commits into
base: development
Choose a base branch
from
2 changes: 2 additions & 0 deletions .github/workflows/compute.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ jobs:
SUPABASE_URL: ${{ secrets.SUPABASE_URL }}
SUPABASE_KEY: ${{ secrets.SUPABASE_KEY }}
VOYAGEAI_API_KEY: ${{secrets.VOYAGEAI_API_KEY}}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

steps:
- uses: actions/checkout@v4
Expand All @@ -46,3 +47,4 @@ jobs:
SUPABASE_URL: ${{ secrets.SUPABASE_URL }}
SUPABASE_KEY: ${{ secrets.SUPABASE_KEY }}
VOYAGEAI_API_KEY: ${{secrets.VOYAGEAI_API_KEY}}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
12 changes: 5 additions & 7 deletions src/adapters/supabase/helpers/comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,10 @@ export class Comment extends SuperSupabase {
//First Check if the comment already exists
const { data, error } = await this.supabase.from("issue_comments").select("*").eq("id", commentNodeId);
if (error) {
this.context.logger.error("Error creating comment", error);
return;
throw new Error("Error creating comment");
}
if (data && data.length > 0) {
this.context.logger.info("Comment already exists");
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to just throw this, then it logs and halts function execution.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure I understand what you're suggesting. Are you asking if I should log a fatal error instead of throwing a new one to halt the function execution?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

You can just throw logger.fatal so you get the log displayed and the error caught on a higher level of execution.

throw new Error("Comment already exists");
return;
} else {
//Create the embedding for this comment
Expand All @@ -47,8 +46,7 @@ export class Comment extends SuperSupabase {
.from("issue_comments")
.insert([{ id: commentNodeId, markdown, plaintext, author_id: authorId, payload, embedding: embedding, issue_id: issueId }]);
if (error) {
this.context.logger.error("Error creating comment", error);
return;
throw new Error("Error creating comment: " + error.message);
}
}
this.context.logger.info("Comment created successfully");
Expand Down Expand Up @@ -80,7 +78,7 @@ export class Comment extends SuperSupabase {
.update({ markdown, plaintext, embedding: embedding, payload, modified_at: new Date() })
.eq("id", commentNodeId);
if (error) {
this.context.logger.error("Error updating comment", error);
throw new Error("Error updating comment: " + error.message);
}
}
}
Expand All @@ -96,7 +94,7 @@ export class Comment extends SuperSupabase {
async deleteComment(commentNodeId: string) {
const { error } = await this.supabase.from("issue_comments").delete().eq("id", commentNodeId);
if (error) {
this.context.logger.error("Error deleting comment", error);
throw new Error("Error deleting comment: " + error.message);
}
}
}
30 changes: 12 additions & 18 deletions src/adapters/supabase/helpers/issues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@ export class Issues extends SuperSupabase {
//First Check if the issue already exists
const { data, error } = await this.supabase.from("issues").select("*").eq("id", issueNodeId);
if (error) {
this.context.logger.error("Error creating issue", error);
return;
throw new Error("Error creating issue: " + error.message);
}
if (data && data.length > 0) {
this.context.logger.info("Issue already exists");
return;
} else {
const embedding = await this.context.adapters.voyage.embedding.createEmbedding(markdown);
const embedding = await this.context.adapters.voyage.embedding.createEmbedding(markdown, "document");
let plaintext: string | null = markdownToPlainText(markdown);
if (isPrivate) {
payload = null;
Expand All @@ -45,8 +44,7 @@ export class Issues extends SuperSupabase {
}
const { error } = await this.supabase.from("issues").insert([{ id: issueNodeId, payload, markdown, plaintext, author_id: authorId, embedding }]);
if (error) {
this.context.logger.error("Error creating issue", error);
return;
throw new Error("Error creating issue: " + error.message);
}
}
this.context.logger.info("Issue created successfully");
Expand All @@ -61,22 +59,21 @@ export class Issues extends SuperSupabase {
plaintext = null;
}
const issues = await this.getIssue(issueNodeId);
if (issues && issues.length == 0) {
if (!issues) {
this.context.logger.info("Issue does not exist, creating a new one");
await this.createIssue(issueNodeId, payload, isPrivate, markdown, authorId);
} else {
const { error } = await this.supabase.from("issues").update({ markdown, plaintext, embedding, payload, modified_at: new Date() }).eq("id", issueNodeId);

if (error) {
this.context.logger.error("Error updating comment", error);
throw new Error("Error updating issue: " + error.message);
}
}
}

async deleteIssue(issueNodeId: string) {
const { error } = await this.supabase.from("issues").delete().eq("id", issueNodeId);
if (error) {
this.context.logger.error("Error deleting comment", error);
throw new Error("Error deleting issue: " + error.message);
}
}

Expand All @@ -86,8 +83,12 @@ export class Issues extends SuperSupabase {
.select("*")
.eq("id", issueNodeId)
.returns<IssueType[]>();
if (error) {
this.context.logger.error("Error getting issue", error);
if (error || !data || data.length === 0) {
if (error) {
this.context.logger.error("Error getting issue", error);
} else {
this.context.logger.error("Error getting issue: No data found");
}
return null;
}
return data;
Expand All @@ -107,11 +108,4 @@ export class Issues extends SuperSupabase {
}
return data;
}

async updatePayload(issueNodeId: string, payload: Record<string, unknown>) {
const { error } = await this.supabase.from("issues").update({ payload }).eq("id", issueNodeId);
if (error) {
this.context.logger.error("Error updating issue payload", error);
}
}
}
11 changes: 11 additions & 0 deletions src/adapters/supabase/helpers/supabase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,15 @@ export class SuperSupabase {
this.supabase = supabase;
this.context = context;
}

async checkConnection(): Promise<boolean> {
const { error } = await this.supabase.from("issues").select("*").limit(1);
// If there's no error, the connection is working
if (!error) {
return true;
} else {
console.log(error);
throw new Error("Error connecting to Supabase or Schema has not been migrated/created");
}
}
}
11 changes: 9 additions & 2 deletions src/handlers/add-comments.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Context } from "../types";
import { CommentPayload } from "../types/payload";
import { addIssue } from "./add-issue";

export async function addComments(context: Context) {
const {
Expand All @@ -17,7 +18,15 @@ export async function addComments(context: Context) {
if (!markdown) {
throw new Error("Comment body is empty");
}
if (context.payload.issue.pull_request) {
throw new Error("Comment is on a pull request");
}
if ((await supabase.issue.getIssue(issueId)) === null) {
await addIssue(context);
}
await supabase.comment.createComment(markdown, nodeId, authorId, payload, isPrivate, issueId);
logger.ok(`Created Comment with id: ${nodeId}`);
logger.ok(`Successfully created comment!`);
} catch (error) {
if (error instanceof Error) {
logger.error(`Error creating comment:`, { error: error, stack: error.stack });
Expand All @@ -27,7 +36,5 @@ export async function addComments(context: Context) {
throw error;
}
}

logger.ok(`Successfully created comment!`);
logger.debug(`Exiting addComments`);
}
41 changes: 20 additions & 21 deletions src/handlers/issue-deduplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,11 @@ export async function issueChecker(context: Context): Promise<boolean> {
issueBody = removeFootnotes(issueBody);
const similarIssues = await supabase.issue.findSimilarIssues(issue.title + removeFootnotes(issueBody), 0.7, issue.node_id);
if (similarIssues && similarIssues.length > 0) {
const matchIssues = similarIssues.filter((issue) => issue.similarity >= context.config.matchThreshold);
const processedIssues = await processSimilarIssues(similarIssues, context, issueBody);
let processedIssues = await processSimilarIssues(similarIssues, context, issueBody);
processedIssues = processedIssues.filter((issue) =>
matchRepoOrgToSimilarIssueRepoOrg(payload.repository.owner.login, issue.node.repository.owner.login, payload.repository.name, issue.node.repository.name)
);
const matchIssues = processedIssues.filter((issue) => parseFloat(issue.similarity) >= context.config.matchThreshold);
if (matchIssues.length > 0) {
logger.info(`Similar issue which matches more than ${context.config.matchThreshold} already exists`);
//To the issue body, add a footnote with the link to the similar issue
Expand All @@ -58,7 +61,7 @@ export async function issueChecker(context: Context): Promise<boolean> {
});
return true;
}
if (similarIssues.length > 0) {
if (processedIssues.length > 0) {
logger.info(`Similar issue which matches more than ${context.config.warningThreshold} already exists`);
await handleSimilarIssuesComment(context, payload, issueBody, issue.number, processedIssues);
return true;
Expand Down Expand Up @@ -130,15 +133,13 @@ function findMostSimilarSentence(issueContent: string, similarIssueContent: stri
return { sentence: mostSimilarSentence, similarity: maxSimilarity, index: mostSimilarIndex };
}

async function handleSimilarIssuesComment(context: Context, payload: IssuePayload, issueBody: string, issueNumber: number, issueList: IssueGraphqlResponse[]) {
const relevantIssues = issueList.filter((issue) =>
matchRepoOrgToSimilarIssueRepoOrg(payload.repository.owner.login, issue.node.repository.owner.login, payload.repository.name, issue.node.repository.name)
);

if (relevantIssues.length === 0) {
context.logger.info("No relevant issues found with the same repository and organization");
}

async function handleSimilarIssuesComment(
context: Context,
payload: IssuePayload,
issueBody: string,
issueNumber: number,
relevantIssues: IssueGraphqlResponse[]
) {
if (!issueBody) {
return;
}
Expand Down Expand Up @@ -183,16 +184,8 @@ async function handleMatchIssuesComment(
context: Context,
payload: IssuePayload,
issueBody: string,
issueList: IssueGraphqlResponse[]
relevantIssues: IssueGraphqlResponse[]
): Promise<string | undefined> {
const relevantIssues = issueList.filter((issue) =>
matchRepoOrgToSimilarIssueRepoOrg(payload.repository.owner.login, issue.node.repository.owner.login, payload.repository.name, issue.node.repository.name)
);

if (relevantIssues.length === 0) {
context.logger.info("No relevant issues found with the same repository and organization");
}

if (!issueBody) {
return;
}
Expand Down Expand Up @@ -292,9 +285,15 @@ export function removeFootnotes(content: string): string {
contentWithoutFootnotes = contentWithoutFootnotes.replace(new RegExp(`\\[\\^${footnoteNumber}\\^\\]`, "g"), "");
});
}
contentWithoutFootnotes = removeCautionMessages(contentWithoutFootnotes);
return contentWithoutFootnotes;
}

export function removeCautionMessages(content: string): string {
const cautionRegex = />[!CAUTION]\n> This issue may be a duplicate of the following issues:\n((> - \[[^\]]+\]\([^)]+\)\n)+)/g;
Copy link
Member

Choose a reason for hiding this comment

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

What are these checks? Seems like wrong approach to be doing this. I would expect you should be able to select the right data in the first place and process less comments with a better strategy, although I don't understand the full context of this process here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These checks are in place to avoid generating embeddings for comment or issue bodies that include footnotes or caution messages.

Copy link
Member

Choose a reason for hiding this comment

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

Just ignore every bot comment is my point

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but when the user updates an issue, it includes footnotes and a caution statement. Should these be propagated to the database as well? I think bot actions are ignored by the kernel.

return content.replace(cautionRegex, "");
}

function checkIfDuplicateFootNoteExists(content: string): boolean {
const footnoteDefRegex = /\[\^(\d+)\^\]: ⚠ \d+% possible duplicate - [^\n]+(\n|$)/g;
const footnotes = content.match(footnoteDefRegex);
Expand Down
7 changes: 7 additions & 0 deletions src/handlers/update-comments.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Context } from "../types";
import { CommentPayload } from "../types/payload";
import { addIssue } from "./add-issue";

export async function updateComment(context: Context) {
const {
Expand All @@ -18,6 +19,12 @@ export async function updateComment(context: Context) {
if (!markdown) {
throw new Error("Comment body is empty");
}
if (context.payload.issue.pull_request) {
throw new Error("Comment is on a pull request");
}
if ((await supabase.issue.getIssue(issueId)) === null) {
await addIssue(context);
}
await supabase.comment.updateComment(markdown, nodeId, authorId, payload, isPrivate, issueId);
} catch (error) {
if (error instanceof Error) {
Expand Down
1 change: 0 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export async function run() {
};

await plugin(inputs, env);

return returnDataToKernel(process.env.GITHUB_TOKEN, inputs.stateId, {});
}

Expand Down
4 changes: 3 additions & 1 deletion src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ export async function runPlugin(context: Context) {
await issueChecker(context);
return await issueMatching(context);
case "issues.edited":
await issueChecker(context);
await updateIssue(context);
await issueChecker(context);
return await issueMatching(context);
case "issues.deleted":
return await deleteIssues(context);
Expand Down Expand Up @@ -72,5 +72,7 @@ export async function plugin(inputs: PluginInputs, env: Env) {
adapters: {} as ReturnType<typeof createAdapters>,
};
context.adapters = createAdapters(supabase, voyageClient, context);
//Check the supabase adapter
await context.adapters.supabase.super.checkConnection();
return await runPlugin(context);
}
19 changes: 19 additions & 0 deletions tests/__mocks__/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ export interface CommentMock {
embedding: number[];
}

export interface IssueMock {
id: string;
plaintext: string;
author_id: number;
payload: Record<string, unknown>;
}

export function createMockAdapters(context: Context) {
const commentMap: Map<string, CommentMock> = new Map();
return {
Expand Down Expand Up @@ -72,6 +79,18 @@ export function createMockAdapters(context: Context) {
return commentMap.get(commentNodeId);
}),
} as unknown as Comment,
issue: {
getIssue: jest.fn(async (issueNodeId: string) => {
return [
{
id: issueNodeId,
plaintext: STRINGS.HELLO_WORLD,
author_id: 1,
payload: {},
} as IssueMock,
];
}),
},
},
voyage: {
embedding: {
Expand Down