Skip to content

Conversation

DevOps-zhuang
Copy link
Contributor

#122
This pull request includes various changes aimed at enhancing the metrics visualization and data validation for GitHub Copilot. The most important changes include updates to the README.md file, refactoring the API calls and data handling, adding a new data conversion utility, and implementing a metrics validator. Additionally, there are updates to the Vue components to improve the display and interaction with the metrics data.

Documentation Updates:

  • README.md: Updated image sources and restructured the key metrics section for better clarity and presentation.

API and Data Handling Improvements:

  • src/api/GitHubApi.ts: Refactored the API calls to handle Copilot metrics, including ensuring data structure consistency and converting metrics data for usage. [1] [2] [3] [4]
  • src/api/MetricsToUsageConverter.ts: Added a new utility to convert Copilot metrics to a structured format suitable for usage analysis.
  • src/api/MetricsValidator.ts: Implemented a validator class to check for continuous dates, validate code completions, and chat engaged users in the metrics data.

Frontend Enhancements:

…ons bug for acceptance rate (by lines); remove unused variables in SeatsAnalysisViewer
…te labels, improve clarity, and reorder cards for better focus on acceptance rates by count
… enhance error handling and improve code readability
…usage route 2) add MetricsToUsageConverter class, to map the new returned metrics schedma to old metrics schema, so the caller don't need to update more. 3) add CopilotUsageChecker to check the data quality of the new fetched schema, and display it in api-response page.
…re, simplify the button layout in the ApiResponse component.
@massimogentilini
Copy link

Tested on my installation with my data, some feedbacks:

  • New breakdown: really useful, but in my scenario the colors of the pie is too similar
    image

  • API analysis, good but from my point of view this error is quite normal because it's normal to do not have data during weekends or festivities. Very minor one indeed, maybe improving the message (do not over analyze them to consider non working days because it's a mess)
    Some metrics are invalid! nonContinuousDates: [ "2024-11-10", "2024-11-26", "2024-11-30" ]

  • In the main page the first two graphs are larger than the other ones, not nice to see.
    image

@DevOps-zhuang
Copy link
Contributor Author

DevOps-zhuang commented Dec 6, 2024 via email

@DevOps-zhuang
Copy link
Contributor Author

Tested on my installation with my data, some feedbacks:

  • New breakdown: really useful, but in my scenario the colors of the pie is too similar
    image
  • API analysis, good but from my point of view this error is quite normal because it's normal to do not have data during weekends or festivities. Very minor one indeed, maybe improving the message (do not over analyze them to consider non working days because it's a mess)
    Some metrics are invalid! nonContinuousDates: [ "2024-11-10", "2024-11-26", "2024-11-30" ]
  • In the main page the first two graphs are larger than the other ones, not nice to see.
    image

thanks, very sueful suggestion and feedback, thanks!
will consider the UI part. and for the API message part, since it is for reference, and it is not easy to consider it is weekend or holiday since holiday is different in differt countries/time difference. I will update the the message accordingly. the main reason is that I found the data is not always right, so I added for visibility and it takes double check. once the metric data is of high quality, will consider remvoe this kind of check :-)

the message will be like as below:
Some metrics are possible invalid! please double check! nonContinuousDates: [ "2024-11-10", "2024-11-26", "2024-11-30" ]

@DevOps-zhuang
Copy link
Contributor Author

Tested on my installation with my data, some feedbacks:

  • New breakdown: really useful, but in my scenario the colors of the pie is too similar
    image
  • API analysis, good but from my point of view this error is quite normal because it's normal to do not have data during weekends or festivities. Very minor one indeed, maybe improving the message (do not over analyze them to consider non working days because it's a mess)
    Some metrics are invalid! nonContinuousDates: [ "2024-11-10", "2024-11-26", "2024-11-30" ]
  • In the main page the first two graphs are larger than the other ones, not nice to see.
    image

Thanks for your comments, I just updated based on your suggestion, can you double check it? thanks.

the color of the pie for new breakdown is as below:
image

@karpikpl karpikpl requested a review from Copilot January 5, 2025 00:33
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 18 out of 28 changed files in this pull request and generated 1 comment.

Files not reviewed (10)
  • src/assets/enterprise_seats_response_sample.json: Language not supported
  • src/assets/organization_seats_response_sample.json: Language not supported
  • src/components/SeatsAnalysisViewer.vue: Evaluated as low risk
  • src/components/ApiResponse.vue: Evaluated as low risk
  • src/components/BreakdownComponent.vue: Evaluated as low risk
  • src/components/MainComponent.vue: Evaluated as low risk
  • src/components/MetricsViewer.vue: Evaluated as low risk
  • src/api/MetricsValidator.ts: Evaluated as low risk
  • src/model/Breakdown.ts: Evaluated as low risk
  • src/model/Copilot_Metrics.ts: Evaluated as low risk

const totalChatCopies = metric.copilot_ide_chat?.editors?.reduce((sum, editor) =>
sum + editor.models?.reduce((sum, model) => sum + model.total_chat_copy_events, 0), 0) || 0;

console.log(`Date: ${metric.date}`);
Copy link
Preview

Copilot AI Jan 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Console logs should be removed or replaced with proper logging mechanisms.

Copilot uses AI. Check for mistakes.

// Get the top 5 breakdowns by acceptance rate
const top5BreakdownsAcceptanceRate = breakdownList.value.slice(0, 5);
// for test, it seems there is an issue in data, so need to get the data in console
console.log('Breakdown List:', JSON.stringify(breakdownList.value, null, 2));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this line still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not needed now, the issue is fixed. in the past, it didn't refresh

@karpikpl
Copy link
Collaborator

karpikpl commented Jan 5, 2025

is this an issue with data when Acceptance is higher than Suggestions?
image

Same with lines suggested/accepted
image

It would be better if we used valid sample data instead of ones that has those abnormalities.

When I checked API response I got

Some metrics maybe are invalid, pls double check! 1). invalidCodeCompletions:

{
        "date": "2024-11-18",
        "editor": "JetBrains",
        "language": "java"
},

but it doesn't say what's wrong, which is little confusing.

<img width="800" alt="image" src="./images/KeyMetrics.png">
</p>

1. **Acceptance Rate:** This metric represents the ratio of accepted numbers to the total numbers suggested by GitHub Copilot. This rate is an indicator of the relevance and usefulness of Copilot's suggestions.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. **Acceptance Rate:** This metric represents the ratio of accepted numbers to the total numbers suggested by GitHub Copilot. This rate is an indicator of the relevance and usefulness of Copilot's suggestions.
1. **Acceptance Rate:** This metric represents the ratio of accepted lines and suggestions to the total suggested by GitHub Copilot. This rate is an indicator of the relevance and usefulness of Copilot's suggestions.
However, as any metric, should be used with caution as developers use Copilot in many different ways (research, confirm, verify etc. not always "inject").

@@ -15,45 +15,49 @@ https://github.com/github-copilot-resources/copilot-metrics-viewer/assets/332930
## Charts

## Key Metrics
Here are the key metrics visualized in these charts:
1. **Acceptance Rate:** This metric represents the ratio of accepted lines to the total lines suggested by GitHub Copilot. This rate is an indicator of the relevance and usefulness of Copilot's suggestions.
Here are the key metrics visualized in these charts,will introduce them one by one as below
Copy link
Collaborator

@karpikpl karpikpl Jan 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Here are the key metrics visualized in these charts,will introduce them one by one as below
>[!NOTE]
> Metrics details are described in detail in [GitHub API response schema](https://docs.github.com/en/rest/copilot/copilot-metrics?apiVersion=2022-11-28#get-copilot-metrics-for-an-organization)
Here are the key metrics visualized in these charts:

required: true
},
metrics: {
type: Array as () => CopilotMetrics[],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type: Array as () => CopilotMetrics[],
type: Array as () => Metrics[],

</v-container>
</template>

<script lang="ts">
import { defineComponent } from 'vue';
import config from '../config';
import { MetricsValidator } from '../api/MetricsValidator';
import { CopilotMetrics } from '../model/Copilot_Metrics';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import { CopilotMetrics } from '../model/Copilot_Metrics';
import { CopilotMetrics } from '../model/Copilot_Metrics';
import { Metrics } from '@/model/Metrics';

@karpikpl karpikpl linked an issue Jan 5, 2025 that may be closed by this pull request
@karpikpl
Copy link
Collaborator

karpikpl commented Jan 5, 2025

@DevOps-zhuang - from what I can tell this also contains #116 and #115 which fixes #113 ?

@DevOps-zhuang
Copy link
Contributor Author

DevOps-zhuang commented Jan 5, 2025 via email

this.message = 'All metrics are valid!';
this.isError = false;
} else {
this.message = 'Some metrics maybe are invalid, pls double check!\n';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.message = 'Some metrics maybe are invalid, pls double check!\n';
this.message = 'Some metrics might be inconsistent, please double check the API response.\n';

Copy link
Collaborator

@karpikpl karpikpl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the API update! I've posted some code suggestions

Comment on lines +213 to +214
// for test, it seems there is an issue in data, so need to get the data in console
console.log('Breakdown List:', JSON.stringify(breakdownList.value, null, 2));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// for test, it seems there is an issue in data, so need to get the data in console
console.log('Breakdown List:', JSON.stringify(breakdownList.value, null, 2));

@karpikpl
Copy link
Collaborator

this has been replaced by #138 which included all the code changes + tests.
Thanks @DevOps-zhuang !

@karpikpl karpikpl closed this Jan 11, 2025
@DevOps-zhuang
Copy link
Contributor Author

DevOps-zhuang commented Jan 11, 2025 via email

karpikpl added a commit that referenced this pull request Jan 13, 2025
…ilot-metrics

Feature/new copilot metrics API - replaces #125
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.

Update API to latest version
3 participants