Skip to content

Commit

Permalink
feat(review): Support step-by-step multi-threaded review (#6)
Browse files Browse the repository at this point in the history 
- Create ChatScript to setup chat instructions
- Fix JSON deserialization
- Support line range on feedback
- Support specifying commit to review
- Increase default Anthropic maximum output tokens
  • Loading branch information
@skarllot
skarllot authored Dec 25, 2024
1 parent c10cf18 commit 9585e43
Show file tree
Hide file tree
Showing 12 changed files with 398 additions and 133 deletions.
93 changes: 93 additions & 0 deletions src/FlowPair/Agent/Operations/ReviewChanges/ChatScript.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
using System.Collections.Immutable;

namespace Ciandt.FlowTools.FlowPair.Agent.Operations.ReviewChanges;

public sealed record ChatScript(
string Name,
ImmutableArray<string> Extensions,
string SystemInstruction,
ImmutableList<Instruction> Instructions)
{
public const string StopKeywordPlaceholder = "<NO FEEDBACK>";
public static readonly ImmutableList<ChatScript> Default =
[
new(
"Generic programming chat script",
[
/* Python */".py", ".pyw", ".pyx", ".pxd", ".pxi",
/* JavaScript */".js", ".jsx", ".mjs", ".cjs",
/* Java */".java",
/* C# */".cs", ".csx",
/* C++ */".cpp", ".cxx", ".cc", ".c++", ".hpp", ".hxx", ".h", ".hh", ".h++",
/* PHP */".php", ".phtml", ".phps",
/* Ruby */".rb", ".rbw", ".rake",
/* Swift */".swift",
/* R */".r",
/* SQL */".sql",
/* Kotlin */".kt", ".kts",
/* TypeScript */".ts", ".tsx",
/* Go (Golang) */".go",
/* Rust */".rs",
/* Scala */".scala", ".sc",
/* Dart */".dart",
/* Perl */".pl", ".pm", ".t", ".pod",
/* MATLAB */".m",
/* VBA */".bas", ".cls", ".frm",
/* Shell Scripting */".sh", ".bash", ".zsh", ".ksh", ".csh", ".tcsh", ".fish",
],
"""
You are an expert developer, your task is to review a set of changes on Git commits.
You are given a set of Git patches, containing the filenames and their partial contents. Note that you might not have the full context of the code.
Only review lines of code which have been changed (added or removed) in the pull request. Other lines are added to provide context but should be ignored in the review.
Begin your feedback by evaluating the changed code using a risk score similar to a LOGAF score but measured from 0 to 3, where 0 is the lowest risk to the codebase if the code is merged and 3 is the highest risk which would likely break something or be unsafe. Risk score should be described as "0 - Not important", "1 - Low priority adjustments", "2 - Medium priority adjustments" or "3 - High priority adjustments".
Only provide feedback on critical issues. If the code is already well-written or issues are minor, do not provide any feedback.
Avoid commenting on breaking functions down into smaller, more manageable functions unless it is a significant problem. Be aware that there will be libraries and techniques used which you might not be familiar with, so do not comment on those unless you are confident that there is a problem.
""",
[
Instruction.MultiStepInstruction.Of(
"Give feedback to ",
[
"improve readability where it can significantly impacts understanding",
"make code cleaner where it introduces substantial benefits",
"maximize the performance of the code where there is a clear, impactful improvement",
"flag any API keys or secrets present in plain text immediately as highest risk",
"rate the changes based on SOLID principles",
"apply the principles of DRY, KISS, YAGNI and Clean Code",
"avoid magic strings and numbers",
"ensure new code follow existing patterns and structure",
],
$" if applicable; otherwise, reply with \"{StopKeywordPlaceholder}\" when there are no suggestions"),
Instruction.StepInstruction.Of(
"""
Ensure the feedback contain the file path and the line number.
Do not provide positive reinforcement or comments on good decisions. Focus solely on areas that need improvement.
"""),
Instruction.StepInstruction.Of(
"""
Ensure the feedback details are brief, concise, and accurate. If there are multiple similar issues, only comment on the most critical.
Include brief example code snippets in the feedback details for your suggested changes when you're confident your suggestions are improvements.
Use the same programming language as the file under review. If there are multiple improvements you suggest in the feedback details, use an ordered list to indicate the priority of the changes.
"""),
Instruction.StepInstruction.Of(
"""
Ensure the message in the feedback is in English.
Ensure the feedback do not infer unknown code, do not speculate the referenced code.
"""),
Instruction.JsonConvertInstruction.Of(
"""
Format the feedback in a valid JSON format as a list of feedbacks, or "[]" for no feedbacks.
The "feedback" property can be multiline and include example code snippets.
The schema of the JSON feedback object must be:
"""),
]),
];

public static Option<ChatScript> FindChatScriptForFile(
IReadOnlyList<ChatScript> scripts,
string filePath)
{
return scripts
.Reverse()
.FirstOrDefault(i => i.Extensions.Any(s => filePath.EndsWith(s, StringComparison.OrdinalIgnoreCase)));
}
}
11 changes: 11 additions & 0 deletions src/FlowPair/Agent/Operations/ReviewChanges/ChatThreadSpec.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using System.Collections.Immutable;
using Ciandt.FlowTools.FlowPair.Flow.Operations.ProxyCompleteChat.v1;

namespace Ciandt.FlowTools.FlowPair.Agent.Operations.ReviewChanges;

public static class ChatThreadSpec
{
public static bool IsClosed(ImmutableList<Message> thread, string stopKeyword) =>
thread[^1].Role == Role.Assistant &&
thread[^1].Content.Contains(stopKeyword, StringComparison.Ordinal);
}
18 changes: 10 additions & 8 deletions src/FlowPair/Agent/Operations/ReviewChanges/ContentDeserializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,35 @@ namespace Ciandt.FlowTools.FlowPair.Agent.Operations.ReviewChanges;

public static class ContentDeserializer
{
public static ImmutableList<ReviewerFeedbackResponse> TryDeserializeFeedback(
public static Result<ImmutableList<ReviewerFeedbackResponse>, string> TryDeserializeFeedback(
ReadOnlySpan<char> content,
JsonTypeInfo<ImmutableList<ReviewerFeedbackResponse>> typeInfo)
{
if (content.IsWhiteSpace())
return [];
return "JSON not found on empty content";

JsonException? lastException = null;
while (!content.IsEmpty)
{
var start = content.IndexOf('[');
if (start < 0)
return [];
return "Invalid JSON: '[' not found";

var end = content.IndexOf(']');
var end = content.LastIndexOf(']');
if (end < start)
return [];
return "Invalid JSON: ']' not found or comes before '['";

try
{
return JsonSerializer.Deserialize(content[start..(end + 1)], typeInfo) ?? [];
}
catch (JsonException)
catch (JsonException exception)
{
content = content[(end + 1)..];
lastException = exception;
content = content[(start + 1)..];
}
}

return [];
return lastException?.Message ?? "Invalid JSON";
}
}
2 changes: 1 addition & 1 deletion src/FlowPair/Agent/Operations/ReviewChanges/FeedbackHtmlTemplate.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public override string TransformText()
#line hidden
this.Write(":");

this.Write(this.ToStringHelper.ToStringWithCulture(response.Line));
this.Write(this.ToStringHelper.ToStringWithCulture(response.LineRange));

#line default
#line hidden
Expand Down
2 changes: 1 addition & 1 deletion src/FlowPair/Agent/Operations/ReviewChanges/FeedbackHtmlTemplate.tt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<h2 class="subtitle">
<span class="tag<#= GetRiskLevelCssClass(response.RiskScore) #>">Risk Score: <#= response.RiskScore #></span> <#= response.RiskDescription #>
</h2>
<p><strong>File:</strong> <#= response.Path #>:<#= response.Line #></p>
<p><strong>File:</strong> <#= response.Path #>:<#= response.LineRange #></p>
<div class="notification">
<#= HtmlFormatter.EncodeToHtml(response.Feedback) #>
</div>
Expand Down
107 changes: 6 additions & 101 deletions src/FlowPair/Agent/Operations/ReviewChanges/Instructions.cs
View file
Original file line number Diff line number Diff line change
@@ -1,109 +1,14 @@
using System.Collections.Immutable;
using FxKit.CompilerServices;

namespace Ciandt.FlowTools.FlowPair.Agent.Operations.ReviewChanges;

public sealed record Instructions(
string Name,
ImmutableArray<string> Extensions,
string Message)
[Union]
public partial record Instruction
{
public static readonly ImmutableList<Instructions> Default =
[
new Instructions(
"Generic programming instructions",
[
/* Python */".py", ".pyw", ".pyx", ".pxd", ".pxi",
/* JavaScript */".js", ".jsx", ".mjs", ".cjs",
/* Java */".java",
/* C# */".cs", ".csx",
/* C++ */".cpp", ".cxx", ".cc", ".c++", ".hpp", ".hxx", ".h", ".hh", ".h++",
/* PHP */".php", ".phtml", ".phps",
/* Ruby */".rb", ".rbw", ".rake",
/* Swift */".swift",
/* R */".r",
/* SQL */".sql",
/* Kotlin */".kt", ".kts",
/* TypeScript */".ts", ".tsx",
/* Go (Golang) */".go",
/* Rust */".rs",
/* Scala */".scala", ".sc",
/* Dart */".dart",
/* Perl */".pl", ".pm", ".t", ".pod",
/* MATLAB */".m",
/* VBA */".bas", ".cls", ".frm",
/* Shell Scripting */".sh", ".bash", ".zsh", ".ksh", ".csh", ".tcsh", ".fish"
],
"""
You are an expert developer, your task is to review a set of pull requests on Azure DevOps.
partial record StepInstruction(string Messsage);

You are given a set of Git patches, containing the filenames and their partial contents. Note that you might not have the full context of the code.
partial record MultiStepInstruction(string Preamble, ImmutableList<string> Messages, string Ending);

Only review lines of code which have been changed (added or removed) in the pull request. Lines which have been removed have the type `REMOVED` and lines which have been added have the type `ADDED`. Other lines are added to provide context but should be ignored in the review.
Begin your feedback by evaluating the changed code using a risk score similar to a LOGAF score but measured from 0 to 3, where 0 is the lowest risk to the codebase if the code is merged and 3 is the highest risk which would likely break something or be unsafe. Risk score should be described as "0 - Not important", "1 - Low priority adjustments", "2 - Medium priority adjustments" or "3 - High priority adjustments".
In your feedback:
1. Focus exclusively on highlighting potential bugs.
2. Improve readability only if it significantly impacts understanding.
3. Make code cleaner only if it introduces substantial benefits.
4. Maximize the performance of the code if there is a clear, impactful improvement.
5. Flag any API keys or secrets present in plain text immediately as highest risk.
6. Rate the changes based on SOLID principles if applicable.
7. Apply the principles of DRY, KISS, YAGNI and Clean Code during the review of the code.
8. Do not infer unknown code, do not speculate the referenced code.
9. Avoid magic strings and numbers.
10. New code should follow existing patterns and structure.
Only provide feedback on critical issues. If the code is already well-written or issues are minor, do not provide any feedback.
Avoid commenting on breaking functions down into smaller, more manageable functions unless it is a significant problem. Be aware that there will be libraries and techniques used which you might not be familiar with, so do not comment on those unless you are confident that there is a problem.
Do not provide positive reinforcement or comments on good decisions. Focus solely on areas that need improvement.
Your feedbacks will be input in Azure DevOps via API `/comments` endpoint. The feedbacks should be in a valid JSON format.
Use markdown formatting for the feedback details. Do not include the filename or risk level in the feedback details.
Ensure the feedback details are brief, concise, and accurate. If there are multiple similar issues, only comment on the most critical.
Include brief example code snippets in the feedback details for your suggested changes when you're confident your suggestions are improvements. Use the same programming language as the file under review. If there are multiple improvements you suggest in the feedback details, use an ordered list to indicate the priority of the changes.
It is not necessary to add low-risk comments that are not relevant to changes in the pull request.
The message in the field text must be in English.
Format the response in a valid JSON format as a list of feedbacks. Remember it is crucial that the result has the file path.
This valid JSON is going to be inserted in a value of a key-value from another JSON object, be-aware about the formatting. Remember to only list feedbacks that needs user action.
The schema of the JSON feedback object must be:
```json
[
{
"riskScore": 0,
"riskDescription": "Not important",
"feedback": "",
"path": "/path/path/file.extension",
"line": 16,
"lineType": "ADDED"
},
{
"riskScore": 1,
"riskDescription": "Low priority adjustments",
"feedback": "",
"path": "/path/path/file.extension",
"line": 20,
"lineType": "ADDED"
}
]
```
""")
];

public static Option<Instructions> FindInstructionsForFile(
IReadOnlyList<Instructions> instructions,
string filePath)
{
return instructions
.Reverse()
.FirstOrDefault(i => i.Extensions.Any(s => filePath.EndsWith(s, StringComparison.OrdinalIgnoreCase)));
}
partial record JsonConvertInstruction(string Message);
}
Loading

0 comments on commit 9585e43

Please sign in to comment.