-
Notifications
You must be signed in to change notification settings - Fork 221
[API View] Feedback mechanism for AI Generated comments #12786
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
base: main
Are you sure you want to change the base?
Changes from all commits
99b2ac3
db837e6
f642729
d6244b5
fbbddce
a9173dd
2e93216
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| using System.Collections.Generic; | ||
|
|
||
| namespace APIViewWeb.LeanModels | ||
| { | ||
| public class CommentFeedbackRequest | ||
| { | ||
| public List<string> Reasons { get; set; } = new(); | ||
| public string Comment { get; set; } = string.Empty; | ||
| public bool IsDelete { get; set; } = false; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,15 @@ public enum CommentSource | |
| Diagnostic | ||
| } | ||
|
|
||
| public class CommentFeedback | ||
| { | ||
| public List<string> Reasons { get; set; } = new List<string>(); | ||
| public string Comment { get; set; } = string.Empty; | ||
| public bool IsDelete { get; set; } | ||
| public string SubmittedBy { get; set; } | ||
| public DateTime? SubmittedOn { get; set; } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this optional? |
||
| } | ||
|
|
||
| public class CommentItemModel | ||
| { | ||
| [System.Text.Json.Serialization.JsonPropertyName("id")] | ||
|
|
@@ -63,6 +72,7 @@ public class CommentItemModel | |
| public List<string> MemoryIds { get; set; } = []; | ||
| public float ConfidenceScore { get; set; } | ||
|
|
||
| public List<CommentFeedback> Feedback { get; set; } = []; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably be nullable. Since it only even applies to AI-generated comments, there's no point storing even empty list for non-AI comments. |
||
| public static CommentSeverity ParseSeverity(string value) | ||
| { | ||
| return value?.ToUpperInvariant() switch | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| <p-dialog | ||
| [(visible)]="visible" | ||
| [modal]="true" | ||
| [closable]="true" | ||
| [resizable]="false" | ||
| [draggable]="false" | ||
| [dismissableMask]="false" | ||
| [header]="dialogTitle" | ||
| [style]="{width: '500px'}" | ||
| appendTo="body" | ||
| (onHide)="onHide()"> | ||
|
|
||
| <div class="dialog-content"> | ||
| <div class="mb-3"> | ||
| <p class="delete-description"> | ||
| {{ dialogDescription }} | ||
| </p> | ||
| </div> | ||
|
|
||
| <!-- Deletion Reason --> | ||
| <div class="deletion-reason mb-3"> | ||
| <label for="deletionReason" class="form-label fw-semibold"> | ||
| Reason for deletion <span class="text-danger">*</span> | ||
| </label> | ||
| <textarea | ||
| id="deletionReason" | ||
| class="form-control" | ||
| [(ngModel)]="reason" | ||
| rows="4" | ||
| placeholder="Explain why this comment is egregiously wrong and must be deleted..." | ||
| [attr.aria-required]="true"> | ||
| </textarea> | ||
| </div> | ||
|
|
||
| <!-- Validation Message --> | ||
| <div *ngIf="!canDelete && reason.length > 0" class="alert alert-warning py-2 px-3 mb-0" role="alert"> | ||
| <i class="bi bi-exclamation-triangle me-2"></i> | ||
| <small>Please provide a reason for deletion.</small> | ||
| </div> | ||
| </div> | ||
|
|
||
| <ng-template pTemplate="footer"> | ||
| <div class="d-flex justify-content-end gap-2 w-100"> | ||
| <button | ||
| type="button" | ||
| class="btn btn-outline-secondary" | ||
| (click)="onCancel()"> | ||
| Cancel | ||
| </button> | ||
| <button | ||
| type="button" | ||
| class="btn btn-danger" | ||
| [disabled]="!canDelete" | ||
| (click)="onDelete()"> | ||
| OK | ||
| </button> | ||
| </div> | ||
| </ng-template> | ||
| </p-dialog> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| .dialog-content { | ||
| .delete-description { | ||
| color: #605e5c; | ||
| font-size: 14px; | ||
| line-height: 1.5; | ||
| margin-bottom: 0; | ||
| } | ||
|
|
||
| .deletion-reason { | ||
| .form-label { | ||
| font-size: 14px; | ||
| margin-bottom: 8px; | ||
| } | ||
|
|
||
| textarea { | ||
| font-size: 14px; | ||
| resize: vertical; | ||
| min-height: 100px; | ||
|
|
||
| &::placeholder { | ||
| color: #a19f9d; | ||
| } | ||
|
|
||
| &:focus { | ||
| border-color: #0078d4; | ||
| box-shadow: 0 0 0 0.2rem rgba(0, 120, 212, 0.25); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| .alert { | ||
| font-size: 13px; | ||
| border-radius: 4px; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | ||
| import { FormsModule } from '@angular/forms'; | ||
| import { NoopAnimationsModule } from '@angular/platform-browser/animations'; | ||
| import { DialogModule } from 'primeng/dialog'; | ||
| import { AICommentDeleteDialogComponent } from './ai-comment-delete-dialog.component'; | ||
|
|
||
| describe('AICommentDeleteDialogComponent', () => { | ||
| let component: AICommentDeleteDialogComponent; | ||
| let fixture: ComponentFixture<AICommentDeleteDialogComponent>; | ||
|
|
||
| beforeEach(() => { | ||
| TestBed.configureTestingModule({ | ||
| declarations: [AICommentDeleteDialogComponent], | ||
| imports: [ | ||
| FormsModule, | ||
| DialogModule, | ||
| NoopAnimationsModule | ||
| ] | ||
| }); | ||
| fixture = TestBed.createComponent(AICommentDeleteDialogComponent); | ||
| component = fixture.componentInstance; | ||
| fixture.detectChanges(); | ||
| }); | ||
|
|
||
| it('should not allow deletion without a reason', () => { | ||
| component.reason = ''; | ||
| expect(component.canDelete).toBeFalsy(); | ||
| }); | ||
|
|
||
| it('should not allow deletion with only whitespace', () => { | ||
| component.reason = ' '; | ||
| expect(component.canDelete).toBeFalsy(); | ||
| }); | ||
|
|
||
| it('should not emit deleteConfirm on delete without reason', () => { | ||
| spyOn(component.deleteConfirm, 'emit'); | ||
| component.commentId = 'test-123'; | ||
| component.reason = ''; | ||
|
|
||
| component.onDelete(); | ||
|
|
||
| expect(component.deleteConfirm.emit).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should reset form after successful delete', () => { | ||
| component.reason = 'This comment is wrong'; | ||
|
|
||
| component.onDelete(); | ||
|
|
||
| expect(component.reason).toBe(''); | ||
| }); | ||
|
|
||
| it('should reset form on cancel', () => { | ||
| component.reason = 'Some reason'; | ||
| component.visible = true; | ||
|
|
||
| spyOn(component.visibleChange, 'emit'); | ||
| spyOn(component.cancel, 'emit'); | ||
|
|
||
| component.onCancel(); | ||
|
|
||
| expect(component.reason).toBe(''); | ||
| expect(component.visible).toBe(false); | ||
| expect(component.visibleChange.emit).toHaveBeenCalledWith(false); | ||
| expect(component.cancel.emit).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should reset form on hide', () => { | ||
| component.reason = 'Some reason'; | ||
|
|
||
| spyOn(component.cancel, 'emit'); | ||
|
|
||
| component.onHide(); | ||
|
|
||
| expect(component.reason).toBe(''); | ||
| expect(component.cancel.emit).toHaveBeenCalled(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| import { Component, EventEmitter, Input, Output } from '@angular/core'; | ||
|
|
||
| export interface AICommentDeleteReason { | ||
| commentId: string; | ||
| reason: string; | ||
| } | ||
|
|
||
| @Component({ | ||
| selector: 'app-ai-comment-delete-dialog', | ||
| templateUrl: './ai-comment-delete-dialog.component.html', | ||
| styleUrls: ['./ai-comment-delete-dialog.component.scss'] | ||
| }) | ||
| export class AICommentDeleteDialogComponent { | ||
| @Input() visible: boolean = false; | ||
| @Input() commentId: string = ''; | ||
| @Output() visibleChange = new EventEmitter<boolean>(); | ||
| @Output() deleteConfirm = new EventEmitter<AICommentDeleteReason>(); | ||
| @Output() cancel = new EventEmitter<void>(); | ||
|
|
||
| reason: string = ''; | ||
|
|
||
| readonly dialogTitle = "Delete Comment"; | ||
| readonly dialogDescription = "Only delete comments that are egregiously wrong or harmful. Please explain why this comment should be removed."; | ||
|
|
||
| get canDelete(): boolean { | ||
| return this.reason.trim().length > 0; | ||
| } | ||
|
|
||
| onDelete(): void { | ||
| if (!this.canDelete) { | ||
| return; | ||
| } | ||
|
|
||
| this.deleteConfirm.emit({ | ||
| commentId: this.commentId, | ||
| reason: this.reason | ||
| }); | ||
|
|
||
| this.closeDialog(); | ||
| } | ||
|
|
||
| onCancel(): void { | ||
| this.cancel.emit(); | ||
| this.closeDialog(); | ||
| } | ||
|
|
||
| onHide(): void { | ||
| this.cancel.emit(); | ||
| this.closeDialog(); | ||
| } | ||
|
|
||
| private closeDialog(): void { | ||
| this.resetForm(); | ||
| this.visible = false; | ||
| this.visibleChange.emit(false); | ||
| } | ||
|
|
||
| private resetForm(): void { | ||
| this.reason = ''; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My gut tells me we should store these as enums, as that will allow copilot to take deterministic action instead of having to parser intent.