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

update pr summary on review_requested #64

Closed
fcakyon opened this issue Jan 24, 2024 · 10 comments · Fixed by #70
Closed

update pr summary on review_requested #64

fcakyon opened this issue Jan 24, 2024 · 10 comments · Fixed by #70
Labels
fixed Bug has been resolved

Comments

@fcakyon
Copy link
Member

fcakyon commented Jan 24, 2024

Check this PR for more detail: safevideo/autollm#223

@SeeknnDestroy
Copy link

SeeknnDestroy commented Jan 24, 2024

we have tried to use

        with:
            token: ${{ secrets.GITHUB_TOKEN }} # automatically generated, do not modify
            summary: true  # print PR summary with GPT4 (requires 'openai_api_key' or 'openai_azure_api_key' and 'openai_azure_endpoint')
            openai_azure_api_key: ${{ secrets.AZURE_API_KEY }}
            openai_azure_endpoint: ${{ secrets.AZURE_API_BASE }}

instead of

        with:
            token: ${{ secrets.GITHUB_TOKEN }} # automatically generated, do not modify
            summary: true  # print PR summary with GPT4 (requires 'openai_api_key' or 'openai_azure_api_key' and 'openai_azure_endpoint')
            openai_azure_api_key: ${{ secrets.OPENAI_AZURE_API_KEY }}
            openai_azure_endpoint: ${{ secrets.OPENAI_AZURE_ENDPOINT }}

with corresponding github secret names. Latter worked, former did not. Why exactly is this the case?

@glenn-jocher
Copy link
Member

@fcakyon @SeeknnDestroy hi there!

So you're saying that you have AZURE_API_KEY and AZURE_API_BASE defined in your org secrets but the upper example is not working for you?

That's very strange, it should work for any secret names you define as long as they are passed in as you've shown above, i.e. this should work as long as they are defined in your org secrets:

        with:
            token: ${{ secrets.GITHUB_TOKEN }} # automatically generated, do not modify
            summary: true  # print PR summary with GPT4 (requires 'openai_api_key' or 'openai_azure_api_key' and 'openai_azure_endpoint')
            openai_azure_api_key: ${{ secrets.CUSTOM_SECRET_NAME1 }}
            openai_azure_endpoint: ${{ secrets.CUSTOM_SECRET_NAME2 }}

@SeeknnDestroy
Copy link

Hi @glenn-jocher,

I just found out the actual problem: it indeed works fine with custom secret names but even though we have type synchronize for triggering action on pull request, it does not create a PR summary for our opened pull request after we push a new commit. It only works when we open or close the PR. Is this a current limitation of this action? Do you plan to work on this?

@glenn-jocher
Copy link
Member

glenn-jocher commented Jan 28, 2024

@SeeknnDestroy oh yes, this is by design, otherwise large PRs may re-summarize on every commit, which would be costly for large works. I'm not sure of an easy way to opt-in to re-summarize, but one hack is to close and re-open the PR.

Due to the non-deterministic nature of LLMs, even with seed 0 the summary may differ significantly for small changes, so i.e. if you have 10 commits and add a new very small change it's likely the entire summary will change significantly.

@fcakyon
Copy link
Member Author

fcakyon commented Jan 28, 2024

@glenn-jocher how aobut pull_request_review: submitted trigger? it would prevent the summary from being updated on each commit. It would update when there is a significant change since the PR creation.

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_review

image

@glenn-jocher
Copy link
Member

Oh yeah this is a great idea! Should be pretty simple, I'll try to get it done a little later.

@glenn-jocher glenn-jocher changed the title pr summary not working with azure endpoint Summary run on review_submitted Jan 29, 2024
@glenn-jocher
Copy link
Member

@fcakyon @SeeknnDestroy ok guys I've updated the action in #70 to run PR summary also when a user requests a review on the PR by adding the review_requested action to the list of actions that will trigger summaries.

I think this should work. What do you think?

@glenn-jocher glenn-jocher linked a pull request Jan 29, 2024 that will close this issue
@glenn-jocher glenn-jocher added the fixed Bug has been resolved label Jan 29, 2024
@glenn-jocher glenn-jocher reopened this Jan 29, 2024
@glenn-jocher
Copy link
Member

Here is example usage. Note that synchronize type is required to run things like Python, Markdown, Spelling, etc.

  pull_request:
    branches: [main]
    # https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
    types: [opened, synchronize, review_requested]

@fcakyon fcakyon changed the title Summary run on review_submitted update pr summary on review_requested Jan 29, 2024
@fcakyon
Copy link
Member Author

fcakyon commented Jan 29, 2024

tested, all working fine! @glenn-jocher

@fcakyon fcakyon closed this as completed Jan 29, 2024
@glenn-jocher
Copy link
Member

@fcakyon awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed Bug has been resolved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants