Adding validation and implementation to minimize comments on github #123
Conversation
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![kodus-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
| @@ -537,7 +537,10 @@ Avoid making assumptions or including inferred details not present in the provid | |||
| ); | |||
| } | |||
|
|
|||
| return `${resultText}\n\n${this.generateConfigReviewMarkdown(organizationAndTeamData, prNumber, codeReviewConfig)}\n\n<!-- kody-codereview -->\n​`; | |||
| // Adicionar tag única com timestamp para identificar este comentário como finalizado | |||
| const uniqueId = `completed-${Date.now()}`; | |||
There was a problem hiding this comment.
const uniqueId = `completed-${Date.now()}-${Math.random().toString(36).slice(2)}`;Two instances were found where Date.now() is used alone to generate unique IDs, which can lead to collisions in high-concurrency scenarios.
This issue appears in multiple locations:
- src/core/infrastructure/adapters/services/codeBase/commentManager.service.ts: Lines 541-541
- src/core/infrastructure/adapters/services/codeBase/commentManager.service.ts: Lines 554-554
Please enhance ID uniqueness by appending a random string (e.g.,Math.random().toString(36).slice(2)) toDate.now()in all ID generation logic.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| prNumber: number, | ||
| repository: { name: string; id: string }, | ||
| platformType: PlatformType, | ||
| ): Promise<{ commentId: string; nodeId?: string } | null>; |
There was a problem hiding this comment.
): Promise<{ commentId: number; nodeId?: string } | null>;The findLastReviewComment method is defined to return a commentId of type string. However, other methods within the ICommentManagerService interface, such as createInitialComment and updateOverallComment, use number for commentId. This type mismatch will likely cause integration issues and runtime errors when these methods are used together. To ensure consistency and prevent bugs, the commentId type should be changed to number.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
src/core/domain/platformIntegrations/interfaces/code-management.interface.ts
Show resolved
Hide resolved
| }, | ||
| }); | ||
|
|
||
| try { |
There was a problem hiding this comment.
try {
await this.commentManagerService.minimizeLastReviewComment(
context.organizationAndTeamData,
context.pullRequest.number,
context.repository,
context.platformType,
);
} catch (error) {
this.logger.warn({
message: `Failed to minimize previous review comment for PR#${context.pullRequest.number}, continuing with new review`,
context: this.stageName,
error: error.message,
metadata: {
organizationAndTeamData: context.organizationAndTeamData,
prNumber: context.pullRequest.number,
},
});
}In the catch block, return context; prematurely exits the executeStage method if minimizing the previous comment fails. This prevents the pipeline from proceeding to create a new review comment, which contradicts the intention stated in the log message: "continuing with new review". The return statement should be removed to allow the stage to continue execution and create the new comment as intended.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
...infrastructure/adapters/services/codeBase/codeReviewPipeline/stages/initial-comment.stage.ts
Outdated
Show resolved
Hide resolved
| this.logger.log({ | ||
| message: `Skipping minimize comment for PR#${prNumber} - platform ${platformType} not supported`, | ||
| context: CommentManagerService.name, | ||
| metadata: { platformType, prNumber }, | ||
| }); |
There was a problem hiding this comment.
this.logger.log({
message: `Skipping minimize comment for PR#${prNumber} - platform ${platformType} not supported`,
context: CommentManagerService.name,
metadata: { platformType, prNumber, organizationAndTeamData },
});According to Kody Rule Tratamento adequado de exceções., all logs should include organizationAndTeamData in their metadata for better traceability. This log, which indicates that a comment minimization is being skipped for an unsupported platform, is missing this crucial data.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| this.logger.log({ | ||
| message: `No previous review comment found to minimize for PR#${prNumber}`, | ||
| context: CommentManagerService.name, | ||
| metadata: { prNumber, repository: repository.name }, | ||
| }); |
There was a problem hiding this comment.
this.logger.log({
message: `No previous review comment found to minimize for PR#${prNumber}`,
context: CommentManagerService.name,
metadata: { prNumber, repository: repository.name, organizationAndTeamData },
});To comply with Kody Rule Tratamento adequado de exceções., organizationAndTeamData must be included in log metadata for traceability. The log for when no previous review comment is found to minimize is missing this information.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const kodyReviewComments = comments.filter(comment => { | ||
| const body = comment.body || ''; | ||
| return ( | ||
| body.includes('<!-- kody-codereview -->') && | ||
| !body.includes('Code Review Started') && // Não é comentário inicial | ||
| ( | ||
| body.includes('Code Review Completed') || | ||
| body.includes('Revisão de Código Concluída') || | ||
| body.includes('Code Review Finalizado') || | ||
| body.includes('<!-- kody-codereview-completed-') // Nova tag única | ||
| ) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
const kodyReviewComments = comments.filter((comment: { body?: string }) => {
const body = comment.body || '';
// The new unique tag is the most reliable way to identify final review comments.
if (body.includes('<!-- kody-codereview-completed-')) {
return true;
}
// Fallback for legacy comments that don't have the unique tag.
return (
body.includes('<!-- kody-codereview -->') &&
!body.includes('Code Review Started') &&
(
body.includes('Code Review Completed') ||
body.includes('Revisão de Código Concluída') ||
body.includes('Code Review Finalizado')
)
);
});The filter logic to find review comments relies on multiple hardcoded, language-specific strings ('Code Review Completed', 'Revisão de Código Concluída', etc.). This approach is brittle and difficult to maintain. The logic should be simplified to prioritize the new, more robust <!-- kody-codereview-completed- tag, using the text-based checks only as a fallback for legacy comments. Additionally, to comply with code standards, the comment parameter in the filter callback should be explicitly typed.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| | 'RESOLVED' | ||
| | 'DUPLICATE' | ||
| | 'SPAM'; | ||
| }): Promise<any | null> { |
There was a problem hiding this comment.
}): Promise<MinimizeCommentResponse> {The function's return type Promise<any | null> is inaccurate because the function throws on error instead of returning null. More importantly, it violates the rule against using any (ID: adcc97a6-27fc-4065-85d9-ab910a68489c). To fix this, the return type should be updated to Promise<MinimizeCommentResponse>, and you should define the MinimizeCommentResponse type based on the shape of the GraphQL mutation's response.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| async minimizeComment( | ||
| params: { | ||
| organizationAndTeamData: OrganizationAndTeamData; | ||
| commentId: string; | ||
| reason?: 'ABUSE' | 'OFF_TOPIC' | 'OUTDATED' | 'RESOLVED' | 'DUPLICATE' | 'SPAM'; | ||
| }, | ||
| type?: PlatformType, | ||
| ): Promise<any | null> { |
There was a problem hiding this comment.
// You should create a specific type for the response, for example:
// type MinimizedCommentResponse = { id: string; minimized: boolean; };
async minimizeComment(
params: {
organizationAndTeamData: OrganizationAndTeamData;
commentId: string;
reason?: 'ABUSE' | 'OFF_TOPIC' | 'OUTDATED' | 'RESOLVED' | 'DUPLICATE' | 'SPAM';
},
type?: PlatformType,
): Promise<MinimizedCommentResponse | null> {The minimizeComment method's return type is Promise<any | null>, which uses the any type. This practice is discouraged as it bypasses TypeScript's static type checking, potentially leading to runtime errors and reducing code maintainability. Please define a specific interface or type for the response object to ensure type safety and make the contract of the function clear.
Kody Rules Violation: Tipagem explícita de parâmetros e retornos
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
Found critical issues please review the requested changes
- The
minimizeCommentmethod should gracefully handle its unsupported status and use a specific return type instead of throwing an error and usingany. - The
commentIdtype should benumberinstead ofstringto be consistent with other methods in the interface. - Add
organizationAndTeamDatato the log metadata for skipped comment minimizations to improve traceability. - Include
organizationAndTeamDatain the log metadata when no previous comment is found to ensure proper traceability.
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
This pull request introduces enhancements to the kodustech/kodus-ai repository by adding validation and implementation to minimize comments on GitHub. Key changes include:
Interface Extensions:
ICommentManagerServiceinterface is extended with two new methods:findLastReviewCommentandminimizeLastReviewCommentto manage review comments on pull requests.ICodeManagementServiceinterface now includes aminimizeCommentmethod with typed parameters, allowing an optionalreasonfor minimizing a comment.Service Implementations:
minimizeCommentmethod is added to theAzureReposService,BitbucketService, andGitlabService. However, these methods are placeholders and currently throw errors, which could lead to runtime issues.GitHubServicenow includes aminimizeCommentmethod to interact with the GitHub API via GraphQL and re-enables logic for handling repositories in personal accounts for OAuth and PAT authentication methods.Code Review Pipeline:
initial-comment.stage.tsto minimize previous review comments on subsequent pipeline runs for GitHub pull requests. However, there is a bug in the error handling that incorrectly terminates the stage.Comment Management:
commentManager.service.tsfile now includes functionality to find and minimize previous review comments on GitHub, aiming to reduce timeline noise. Unique identifiers are added to review comments, with suggestions for improving maintainability and error handling.Platform Integration:
CodeManagementServicenow features aminimizeCommentmethod that delegates calls to the appropriate platform-specific service implementation.These changes aim to streamline comment management and reduce noise in pull request timelines, although some areas require further refinement to prevent potential runtime issues.