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

Add support for markers in description #273

Merged
merged 1 commit into from
Sep 14, 2023
Merged

Conversation

tjwp
Copy link
Contributor

@tjwp tjwp commented Sep 4, 2023

Addresses #213

This change adds support for short-codes or markers in pull request descriptions. The use of these markers works well in combination with a pull request template on GitHub.

This functionality is added to the existing /describe command, but is configurable. I have not made it the default behavior here (use_description_markers=false) but I have configured this mode to be used by default in my organization.

While I was implementing this, I also made the labels optional for the describe command (defaults to public_labels=true). One of my users asked to exclude the header that I included in the short-code expansion so that is also configurable but enabled by default (include_generated_by_header=true).

There is no change to the prompting of the LLM model in the way that this is implemented. It only applies the existing description results to the pull request in a different way.

Example output for this PR

Summary

🤖 Generated by PR Agent at 746140b

This PR introduces support for description markers in PR Agent. These markers are short-codes that can be used in pull request descriptions, and are particularly useful when combined with a pull request template on GitHub. The PR also makes labels optional for the describe command and allows users to exclude the header included in the short-code expansion. The changes are configurable and do not alter the prompting of the LLM model.

Walkthrough

🤖 Generated by PR Agent at 746140b

  • pr_agent/tools/pr_description.py: The main changes in this file include the addition of support for description markers. The run method has been updated to prepare data and types based on the prediction, and to prepare marker replacements if the use_description_markers setting is enabled. The _prepare_prediction method has been updated to return None if use_description_markers is enabled and pr_agent: is not in the user description. New methods _prepare_data, _prepare_types, and _prepare_marker_replacements have been added to handle the new functionality. The _prepare_pr_answer method has been updated to prepare the PR answer based on the AI prediction data.
  • pr_agent/settings/configuration.toml: New settings have been added to the pr_description section: publish_labels, use_description_markers, and include_generated_by_header. These settings control whether labels are published, whether description markers are used, and whether a generated by header is included, respectively.

@mrT23
Copy link
Collaborator

mrT23 commented Sep 6, 2023

thanks @tjwp, i will have time to review this tommorow

@mrT23
Copy link
Collaborator

mrT23 commented Sep 7, 2023

/describe --pr_description.publish_description_as_comment=true

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Title

Implementing Support for Description Markers in PR Agent


PR Type:

Enhancement


PR Description:

This PR introduces the ability to use description markers or short-codes in PR descriptions, which can be particularly useful when combined with a PR template on GitHub. The PR also provides the option to make labels optional for the describe command and allows users to exclude the header included in the short-code expansion. These changes are configurable and do not alter the prompting of the LLM model.


PR Main Files Walkthrough:

pr_agent/tools/pr_description.py: The main changes in this file include the addition of support for description markers. The run method has been updated to prepare data and types based on the prediction, and to prepare marker replacements if the use_description_markers setting is enabled. The _prepare_prediction method has been updated to return None if use_description_markers is enabled and pr_agent: is not in the user description. New methods _prepare_data, _prepare_types, and _prepare_marker_replacements have been added to handle the new functionality. The _prepare_pr_answer method has been updated to prepare the PR answer based on the AI prediction data.
pr_agent/settings/configuration.toml: New settings have been added to the pr_description section: publish_labels, use_description_markers, and include_generated_by_header. These settings control whether labels are published, whether description markers are used, and whether the generated by header is included, respectively.

@mrT23
Copy link
Collaborator

mrT23 commented Sep 7, 2023

@tjwp If i understood correctly, you are implementing here another variant of the 'describe' post-processing output, tailored to you and your team's needs.

Your output looks nice, and I think having two flavors of this important feature is reasonable, and the (advanced) user can pick his favorite one. I might copy some of your design choices also to the default option :-)

However, I have some concerns\comments:

  1. Your post-processing is more tailored to the specific structure of the current prompt. Any change to the prompt will probably break it. Tests would help prevent that

  2. This method is not scalable. If another team wants another post process, we will need to add the main code a third one. and a fourth one. and so on.
    Maybe not in this PR, but the ability to inject an arbitrary post-process from an external file would enable this to be scalable.

  3. Notice that we added the option to also edit the prompt (by using an external file):
    https://github.com/Codium-ai/pr-agent/blob/main/Usage.md#editing-the-prompts
    This might also enable your team to further customize this tool for their needs.

  4. your implementation did not support 'publish_description_as_comment' option. see here:
    145b5db
    for fixing it

@tjwp
Copy link
Contributor Author

tjwp commented Sep 11, 2023

@tjwp If i understood correctly, you are implementing here another variant of the 'describe' post-processing output, tailored to you and your team's needs.

I'll describe more below but I think that the markers + templating provide a more flexible way to think about how the LLM results are presented without users needing write post-processors, only templates. GitHub pull request templates already provide a convenient way to specify those templates per-repo.

  1. Your post-processing is more tailored to the specific structure of the current prompt. Any change to the prompt will probably break it. Tests would help prevent that

Tests would help with that but they still would not account for variability in the response from the LLM. The handling of the keys in the response could be more flexible than the exact matching that I used but my experience in testing with GPT-4 was that the keys were reliable.

  1. This method is not scalable. If another team wants another post process, we will need to add the main code a third one. and a fourth one. and so on.
    Maybe not in this PR, but the ability to inject an arbitrary post-process from an external file would enable this to be scalable.

Another way of looking at this is that it is defining an intermediate structure that is is then injected into a template to produce the final output. From that perspective maybe additional post processors are not needed. It may be that over time additional markers are added and supported. For example, if there were a pr_agent:types marker supported then the current default description could be thought of as a template:

## PR Type:
pr_agent:pr_type

## PR Description:
pr_agent:summary

## PR Walkthrough:
pr_agent:walkthrough

From that perspective, the markers would always be used and the template above would be applied if the current description is empty.

I didn't implement it this way to minimize change to the current behavior, but a next step could be to fully convert to a template.

  1. Notice that we added the option to also edit the prompt (by using an external file):
    https://github.com/Codium-ai/pr-agent/blob/main/Usage.md#editing-the-prompts
    This might also enable your team to further customize this tool for their needs.

We haven't need to modify the prompts too much but I will keep that in mind. As noted in the documentation, the prompt has to conform to the internal interface that provides the expected keys in the output. In that way, I think it is compatible with the marker-centric approach where keys essentially map to specific markers, with some light formatting e.g. for the walkthrough.

  1. your implementation did not support 'publish_description_as_comment' option. see here:
    145b5db
    for fixing it

Thanks for fixing that! I've never use the publish as comment option so it wasn't on my radar.

@mrT23 mrT23 merged commit 746140b into Codium-ai:main Sep 14, 2023
2 checks passed
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.

2 participants