Feat/pull precheck #11
Conversation
Unused files (3)
Unused dependencies (1)
Unused types (1)
|
|
current QA: ubq-testing#5 (comment) I'm targeting this PR and using the spec that is written for it as the baseline for QA. |
src/handlers/pull-precheck.ts
Outdated
| const issueNumber = linkedViaBodyHash[0].replace("#", ""); | ||
| const issue = await fetchIssue({ context, owner: repoOwner, repo: repoName, issueNum: Number(issueNumber) }); | ||
| if (!issue) { | ||
| throw logger.error("This pull request does not have an linked task, please link one before merging."); |
There was a problem hiding this comment.
I've inadvertently implemented a standard that we enforce every single PR must have a linked task, a happy byproduct I'd imagine, should I keep this?
- merging to main, we bypass
- any PR opened by a non-human contributor, we bypass
My spelling these days lmao, I realise the grammar error in these logs and will fix
There was a problem hiding this comment.
There have been exceptions that come to mind. Usually with collaborators but occasionally with proactive contributors. This should not be enforced.
|
Bulk of the logic for this is written, it's refining the prompts that's left now. Previously you have had prompt structures in mind especially for formatting the context window, do you have anything in mind for this feature? |
src/handlers/llm-query-output.ts
Outdated
| throw logger.error(`No answer from OpenAI`); | ||
| } | ||
| logger.info(`Answer: ${answer}`, { tokenUsage }); | ||
| const tokens = `\n\n<!--\n${JSON.stringify(tokenUsage, null, 2)}\n--!>`; |
There was a problem hiding this comment.
We have some means to add structured metadata via a method from the SDK
src/handlers/pull-precheck.ts
Outdated
| const issueNumber = linkedViaBodyHash[0].replace("#", ""); | ||
| const issue = await fetchIssue({ context, owner: repoOwner, repo: repoName, issueNum: Number(issueNumber) }); | ||
| if (!issue) { | ||
| throw logger.error("This pull request does not have an linked task, please link one before merging."); |
There was a problem hiding this comment.
There have been exceptions that come to mind. Usually with collaborators but occasionally with proactive contributors. This should not be enforced.
src/handlers/pull-precheck.ts
Outdated
| } | ||
|
|
||
| if (!taskSpec) { | ||
| throw logger.error("Task Spec not found, please link one before merging."); |
There was a problem hiding this comment.
| throw logger.error("Task Spec not found, please link one before merging."); | |
| throw logger.error("Task specification not found, please link one before merging."); |
Unnecessary shorthand
src/handlers/pull-precheck.ts
Outdated
| throw logger.error("PR Diff not found"); | ||
| } | ||
|
|
||
| const question = "What's missing compared to the spec?"; |
There was a problem hiding this comment.
| const question = "What's missing compared to the spec?"; | |
| const question = "Does the code diff implement all of the required changes per the specification?"; |
Not sure the context of how this is used but i would also go on to provide instructions for how it should offer feedback.
There was a problem hiding this comment.
I haven't started refining the actual review prompt or anything close to it yet so this is not implemented yet
chat history = System Message + user question (sysMsg = "you review code..", query = this string in question) + spec and diff
| @@ -0,0 +1 @@ | |||
| export type CodeReviewStatus = "APPROVE" | "REQUEST_CHANGES" | "COMMENT"; | |||
There was a problem hiding this comment.
Is it really necessary that it needs its own file
There was a problem hiding this comment.
If by the time I'm done it's still solo I'll group it but I expect to fill more types here
I'm not sure what you're referring to. But its important to use clear headers to divide the sections. |
ubiquity-os/plugins-wishlist#29 (comment) - You were very specific and intentional when I implemented |
|
Didn't realise I hadn't pushed code from 10 days ago and left a comment on So anyway tools need an interface that's going to be shared across all completions not just the chatbot so whos designing that interface because there are two PRs now which have set out to build one obv there cannot be two. I feel like it's probably best that @sshivaditya2019 continues to build it maybe and I'll use it? Idk anymore really but lmk if I should build it or wait until #31 has been merged. Thanks. P.S: I've pulled in #28 since it appears to have been approved |
Resolves ubiquity-os/plugins-wishlist#45
Requires #9
Requires #14
TODO