Addition of pull-precheck logic

[ishowvel]

Resolves #45

[ishowvel]

Any code changes you guys may want?
I am waiting for input by all the reviewers before making any changes to avoid any conflicts

[ishowvel]

@0x4007 i think using Claude's 3.5 sonnet is a bad choice as a model to review pull requests as it doesn't support structured outputs, structured outputs enable a model to output a JSON typed object every time.
A better choice might be GPT 4o (2.5$ per million tokens) or OpenAI o1 (15$ per million tokens)

I just tried using https://openrouter.ai/google/gemini-2.0-flash-thinking-exp:free which worked wonderfully

[0x4007]

Then study cline's implementation.

Claude is much better dealing with code changes and it's used in similar tools, like cline.

No chance we are using Gemini I've never heard any compliments on it in the context of dealing with code.

[ishowvel]

The current implementation can and will generate proper json outputs but my thinking was it would have been better to use a model which supported structured outputs.

[gentlementlegen]

I was testing the plugin and noticed that if a pull-request is marked ready for review, the bot posts an error message and marks the review back to draft. While I think this can be a good behavior for outside contributors, I believe this should be overridden by the core team, because sometimes we just bump packages or do quick fixes that do not relate to any tasks. @0x4007 Thoughts?

[gentlementlegen]

Also, I tried to use this with a chat-gpt key and endpoint because I do not have an OpenAI at the moment, and I got the error The token limits for configured model gpt-4o was not found. If this is meant only to support Claude, then do not make it configurable, or make it support any endpoint and model.

[ishowvel]

Also, I tried to use this with a chat-gpt key and endpoint because I do not have an OpenAI at the moment, and I got the error The token limits for configured model gpt-4o was not found. If this is meant only to support Claude, then do not make it configurable, or make it support any endpoint and model.

You can easily add the token limits for said model, I made it configurable so that future support for future claude models becomes easier
Here is how ishowvel@d6a2ba1

[gentlementlegen]

This seems to be hard-coded within the plugin so any modification implies re-compiling it, which doesn't seem very intuitive.

[ishowvel]

This seems to be hard-coded within the plugin so any modification implies re-compiling it, which doesn't seem very intuitive.

i am gonna add this in the config, it would be like tokenLimits: {"anthropic/claude-3.5-sonnet": 200000, gpt-4o: 128000}

[whilefoo]

I was testing the plugin and noticed that if a pull-request is marked ready for review, the bot posts an error message and marks the review back to draft. While I think this can be a good behavior for outside contributors, I believe this should be overridden by the core team, because sometimes we just bump packages or do quick fixes that do not relate to any tasks. @0x4007 Thoughts?

Currently if someone who is not the PR author converts it to ready for review, the bot will skip the review but as you said a lot of times the PR can be created by the core team for quick fixes.
We should disable reviews for core team or alternatively only make a review when the PR is created but if it's converted to ready for review by the core team member (even if it's the PR author) it should not interfere

[0x4007]

Can you show me the link to your QA pull @gentlementlegen easier for me to follow along what the problem is

[ishowvel]

Also heres the qa using meta-llama/llama-3.1-70b-instruct
ishowvel-org/.ubiquity-os#69
ishowvel-org/.ubiquity-os#72

[0x4007]

The "This pull request has passed the automated review, a reviewer will review this pull request shortly" seems unnecessary.

You're not handling the error "unexpected no response from LLM"

[ishowvel]

The "This pull request has passed the automated review, a reviewer will review this pull request shortly" seems unnecessary.

Changing it to "Pull request has passed the automated review"

You're not handling the error "unexpected no response from LLM"

That's the error when openrouter gets a resource exhausted error which gives no response from the llm. I'll change it so that it also outputs the error which caused no output

[0x4007]

I am thinking of how we can indicate that the review has passed without cluttering the timeline with comments.

I want to show that the checking logic ran successfully but I can imagine that will get repetitive and verbose as they continue pushing commits.

[whilefoo]

I am thinking of how we can indicate that the review has passed without cluttering the timeline with comments.

I want to show that the checking logic ran successfully but I can imagine that will get repetitive and verbose as they continue pushing commits.

Just approved review with no comment should be enough. Logic is only ran when it's converted from draft to ready for review and not on every commit

[0x4007]

The possible problem with an approval is that it will imply that the pull can be merged. It is intended to act as a "pre-check" and not imply to the assignee that their work has been accepted.

Furthermore we have the daemon-merging plugin that would automatically merge it in after a week even without human review.

Maybe we can emoji react a thumbs up on the pull body to indicate a successful run. 👍