-
Notifications
You must be signed in to change notification settings - Fork 477
Add PCW validation in CharactersLogger tests #7879
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -33,6 +33,87 @@ describe('CharactersLogger', () => { | |
| let mockActiveTextEditor: Writable<vscode.TextEditor> | undefined | ||
| let mockTextEditorSelectionEvent: vscode.TextEditorSelectionChangeEvent | ||
|
|
||
| // Helper functions to work with telemetry events | ||
| function findCodyCharactersFlushEvent() { | ||
| const event = recordSpy.mock.calls.find( | ||
| call => call[0] === 'cody.characters' && call[1] === 'flush' | ||
| ) | ||
| return event?.[2] | ||
| } | ||
|
|
||
| // Helper functions for PCW tests | ||
| async function setupBasicCodeEnvironment() { | ||
| // Set up the test environment | ||
| onDidChangeActiveTextEditor(mockActiveTextEditor) | ||
| onDidChangeTextEditorSelection(mockTextEditorSelectionEvent) | ||
| vi.resetAllMocks() | ||
|
|
||
| // Human writes code | ||
| const humanCode = 'function humanWrittenCode() {}' | ||
| vi.spyOn(codeBlockUtils, 'isCodeFromChatCodeBlockAction').mockResolvedValueOnce(null) | ||
| await onDidChangeTextDocument( | ||
| createChange({ text: humanCode, range: range(0, 0, 0, 0), rangeLength: 0 }) | ||
| ) | ||
|
|
||
| // Cody writes code | ||
| const codyCode = 'function codyGeneratedCode() {}' | ||
| vi.spyOn(codeBlockUtils, 'isCodeFromChatCodeBlockAction').mockResolvedValueOnce({ | ||
| operation: 'insert', | ||
| code: codyCode, | ||
| lineCount: 1, | ||
| charCount: codyCode.length, | ||
| eventName: 'insert', | ||
| source: 'chat', | ||
| }) | ||
|
|
||
| await onDidChangeTextDocument( | ||
| createChange({ text: codyCode, range: range(1, 0, 1, 0), rangeLength: 0 }) | ||
| ) | ||
|
|
||
| // Advance time to trigger the flush | ||
| vi.advanceTimersByTime(LOG_INTERVAL) | ||
|
|
||
| return { humanCode, codyCode } | ||
| } | ||
|
|
||
| function calculateHumanInsertions(flushEvent: any): number { | ||
| // Get total human insertions across all specific categories from the telemetry event | ||
| // Include stale categories in our calculation to properly count all human edits | ||
| return [ | ||
| // All rapid change categories | ||
| flushEvent.metadata.rapid_m_change_inserted || 0, | ||
| flushEvent.metadata.rapid_s_change_inserted || 0, | ||
| flushEvent.metadata.rapid_xs_change_inserted || 0, | ||
| flushEvent.metadata.rapid_xxs_change_inserted || 0, | ||
| flushEvent.metadata.rapid_xxxs_change_inserted || 0, | ||
| // All regular change categories | ||
| flushEvent.metadata.m_change_inserted || 0, | ||
| flushEvent.metadata.s_change_inserted || 0, | ||
| flushEvent.metadata.xs_change_inserted || 0, | ||
| flushEvent.metadata.xxs_change_inserted || 0, | ||
| flushEvent.metadata.xxxs_change_inserted || 0, | ||
| // Stale changes (including the second edit) | ||
| flushEvent.metadata.stale_xs_change_inserted || 0, | ||
| flushEvent.metadata.stale_xxs_change_inserted || 0, | ||
| flushEvent.metadata.stale_s_change_inserted || 0, | ||
| // Unexpected changes | ||
| flushEvent.metadata.unexpected_inserted || 0, | ||
| ].reduce((sum, value) => sum + value, 0) | ||
| } | ||
|
|
||
| function calculatePCW( | ||
| codyInsertions: number, | ||
| codyDeletions: number, | ||
| humanInsertions: number | ||
| ): number { | ||
| // PCW = (Cody code written - Cody code removed) / Total code written | ||
| const codyNetContribution = codyInsertions - codyDeletions | ||
| const totalCodeWritten = humanInsertions + codyInsertions | ||
|
Contributor
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. Just cross checking if Not related to this PR but thinking out loud and cross checking if this makes sense in general: |
||
|
|
||
| // Avoid division by zero and ensure PCW is calculated correctly | ||
| return totalCodeWritten > 0 ? codyNetContribution / totalCodeWritten : 0 | ||
| } | ||
|
|
||
| beforeEach(() => { | ||
| vi.useFakeTimers() | ||
|
|
||
|
|
@@ -317,4 +398,164 @@ describe('CharactersLogger', () => { | |
| }), | ||
| }) | ||
| }) | ||
|
|
||
| it('should calculate baseline PCW correctly', async () => { | ||
| // Set up basic environment with human and Cody code | ||
| await setupBasicCodeEnvironment() | ||
|
|
||
| // Add one more human edit | ||
| const moreHumanCode = 'const x = 42;' | ||
| vi.spyOn(codeBlockUtils, 'isCodeFromChatCodeBlockAction').mockResolvedValueOnce(null) | ||
| await onDidChangeTextDocument( | ||
| createChange({ text: moreHumanCode, range: range(2, 0, 2, 0), rangeLength: 0 }) | ||
| ) | ||
|
|
||
| // Flush the log again to include the additional human code | ||
| vi.advanceTimersByTime(LOG_INTERVAL) | ||
|
|
||
| // Verify the telemetry event was recorded | ||
| expect(recordSpy).toHaveBeenCalledWith('cody.characters', 'flush', expect.anything()) | ||
| const flushEvent = findCodyCharactersFlushEvent() | ||
|
|
||
| // Verify some human code was tracked in at least one category | ||
| const hasHumanCode = Object.entries(flushEvent.metadata).some(([key, value]) => { | ||
| return ( | ||
| (key.includes('_change') || key.includes('rapid_')) && | ||
| !key.includes('cody_') && | ||
| typeof value === 'number' && | ||
| value > 0 | ||
| ) | ||
| }) | ||
| expect(hasHumanCode).toBe(true) | ||
|
|
||
| // Check by combining ALL telemetry flush events | ||
| const allFlushEvents = recordSpy.mock.calls | ||
| .filter(call => call[0] === 'cody.characters' && call[1] === 'flush') | ||
| .map(call => call[2]) | ||
|
|
||
| // Calculate totals across all events | ||
| let totalHumanInsertions = 0 | ||
| let totalCodyInsertions = 0 | ||
|
|
||
| // Calculate the combined total of human and Cody contributions across all events | ||
| for (const event of allFlushEvents) { | ||
| const humanInserts = calculateHumanInsertions(event) | ||
| const codyInserts = event.metadata.cody_chat_inserted || 0 | ||
|
|
||
| totalHumanInsertions += humanInserts | ||
| totalCodyInsertions += codyInserts | ||
| } | ||
|
|
||
| // Calculate true PCW using all events | ||
| const PCW = totalCodyInsertions / (totalHumanInsertions + totalCodyInsertions) | ||
|
Contributor
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. Just cross checking since its insertion, we are not taking deletions into account no |
||
|
|
||
| // The PCW considering all events should be 31/74(31cody+43human) ≈ 0.419 | ||
| expect(PCW).toBeCloseTo(31 / 74, 3) | ||
| }) | ||
|
|
||
| it('should update PCW correctly when human deletes Cody-generated code', async () => { | ||
| // Set up basic environment with human and Cody code | ||
| await setupBasicCodeEnvironment() | ||
| expect(recordSpy).toHaveBeenCalledWith('cody.characters', 'flush', expect.anything()) | ||
| const telemetryEvent = findCodyCharactersFlushEvent() | ||
|
|
||
| // Capture initial metrics | ||
| const humanInsertions = calculateHumanInsertions(telemetryEvent) | ||
| const initialCodyInsertions = telemetryEvent.metadata.cody_chat_inserted | ||
| const initialPcw = calculatePCW(initialCodyInsertions, 0, humanInsertions) | ||
|
|
||
| // Reset for testing deletion | ||
| recordSpy.mockClear() | ||
|
|
||
| const humanDeletedCodyCode = 10 // 10 chars will be deleted | ||
| vi.spyOn(codeBlockUtils, 'isCodeFromChatCodeBlockAction').mockResolvedValueOnce(null) | ||
| await onDidChangeTextDocument( | ||
| createChange({ | ||
| text: '', | ||
| range: range(1, 0, 1, humanDeletedCodyCode), | ||
| rangeLength: humanDeletedCodyCode, | ||
| }) | ||
| ) | ||
|
|
||
| vi.advanceTimersByTime(LOG_INTERVAL) | ||
|
|
||
| // Verify deletion is tracked in telemetry | ||
| expect(recordSpy).toHaveBeenCalledWith('cody.characters', 'flush', expect.anything()) | ||
| const deleteEventPayload = findCodyCharactersFlushEvent() | ||
|
|
||
| // Sum up all deleted character counts across all categories | ||
| const totalDeleted = Object.entries(deleteEventPayload.metadata) | ||
| .filter(([key]) => key.endsWith('_deleted')) | ||
| .reduce((sum, [_, value]) => sum + (value as number), 0) | ||
|
|
||
| // Verify we recorded the 10 deleted characters somewhere | ||
| expect(totalDeleted).toBe(humanDeletedCodyCode) | ||
|
|
||
| // Calculate updated PCW after human deletion of Cody code | ||
| const updatedCodyNetContribution = initialCodyInsertions - totalDeleted | ||
| const updatedTotalCodeWritten = humanInsertions + initialCodyInsertions - totalDeleted | ||
| const updatedPcw = updatedCodyNetContribution / updatedTotalCodeWritten | ||
|
|
||
| // The PCW should be decreased since human deleted Cody's code | ||
| expect(updatedPcw).toBeLessThan(initialPcw) | ||
|
|
||
| // expect PCW with specific value | ||
| expect(updatedPcw).toBeCloseTo(updatedCodyNetContribution / updatedTotalCodeWritten, 3) | ||
| }) | ||
|
|
||
| it('should update PCW correctly when Cody deletes its own code', async () => { | ||
| // Set up basic environment with human and Cody code | ||
| await setupBasicCodeEnvironment() | ||
| expect(recordSpy).toHaveBeenCalledWith('cody.characters', 'flush', expect.anything()) | ||
| const telemetryEvent = findCodyCharactersFlushEvent() | ||
|
|
||
| // Capture initial metrics | ||
| const humanInsertions = calculateHumanInsertions(telemetryEvent) | ||
| const initialCodyInsertions = telemetryEvent.metadata.cody_chat_inserted | ||
| const initialPcw = calculatePCW(initialCodyInsertions, 0, humanInsertions) | ||
|
|
||
| // Reset for testing Cody's self-deletion | ||
| recordSpy.mockClear() | ||
|
|
||
| // Simulate Cody deleting some of its own code | ||
| const codySelfDeletion = 15 // Cody deletes 15 characters of its own code | ||
|
|
||
| // Mock the document change type classifier to attribute the deletion to Cody | ||
| const getDocumentChangeTypeSpy = vi.spyOn(tracker, 'getDocumentChangeType' as any) | ||
| getDocumentChangeTypeSpy.mockImplementationOnce(() => 'cody_chat') | ||
|
|
||
| // Simulate Cody deleting its own code through the normal event flow | ||
| await onDidChangeTextDocument( | ||
| createChange({ | ||
| text: '', | ||
| range: range(1, 0, 1, codySelfDeletion), | ||
| rangeLength: codySelfDeletion, | ||
| }) | ||
| ) | ||
|
|
||
| vi.advanceTimersByTime(LOG_INTERVAL) | ||
|
|
||
| // Verify Cody's self-deletion is tracked in telemetry | ||
| expect(recordSpy).toHaveBeenCalledWith('cody.characters', 'flush', expect.anything()) | ||
| const codySelfDeletionPayload = findCodyCharactersFlushEvent() | ||
|
|
||
| // Verify cody_chat_deleted is populated with correct deletion count | ||
| expect(codySelfDeletionPayload.metadata.cody_chat_deleted).toBe(codySelfDeletion) | ||
|
|
||
| // Get values from telemetry | ||
| const finalCodyInsertions = | ||
| codySelfDeletionPayload.metadata.cody_chat_inserted || initialCodyInsertions | ||
| const finalCodyDeletions = codySelfDeletionPayload.metadata.cody_chat_deleted | ||
|
|
||
| // Calculate PCW after Cody's self-deletion | ||
| const finalPcw = calculatePCW(finalCodyInsertions, finalCodyDeletions, humanInsertions) | ||
|
|
||
| // PCW should be decreased after Cody deletes its own code | ||
| expect(finalPcw).toBeLessThan(initialPcw) | ||
|
|
||
| // expect PCW with specific value - using the finalPcw formula directly | ||
| const calculatedPcw = | ||
| (finalCodyInsertions - finalCodyDeletions) / (humanInsertions + finalCodyInsertions) | ||
| expect(finalPcw).toBeCloseTo(calculatedPcw, 3) | ||
| }) | ||
| }) | ||
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.
cross checking if
flushEventshould have any type apart from any