Add `resolved_issue_size_distribution` field by Goder-0 · Pull Request #238 · aicers/github-dashboard-server

Goder-0 · 2025-09-05T06:58:44Z

Close #197

Graphql Response

codecov · 2025-09-05T06:59:47Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 35.51%. Comparing base (4a60da4) to head (0ddcfec).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #238      +/-   ##
==========================================
+ Coverage   34.42%   35.51%   +1.08%     
==========================================
  Files          17       17              
  Lines        1008     1025      +17     
==========================================
+ Hits          347      364      +17     
  Misses        661      661

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

danbi2990

Could you add a changelog entry for the new API?

danbi2990 · 2025-09-08T01:50:23Z

+impl TryFrom<&str> for IssueSize {
+    type Error = ();
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {


How about using the from trait, because we already added a None variant for exceptions?

Thanks for the suggestion. I’ve applied it.

danbi2990 · 2025-09-08T01:51:41Z

+                    item.project_title == super::issue::TODO_LIST_PROJECT_TITLE
+                        && item.todo_status.as_deref() == Some(super::issue::TODO_LIST_STATUS_DONE)


This seems redundant because the issues are already validated as resolved.

Removed it.

danbi2990 · 2025-09-08T01:53:54Z

+                .map_or(IssueSize::None, |item| {
+                    item.todo_size
+                        .as_deref()
+                        .and_then(|s| IssueSize::try_from(s).ok())


How about creating a HashMap<&str, usize> first and then converting it to a Vec<IssueSize>? This would allow IssueSize to drop the hash derive.

Thanks for the suggestion. I’ve applied it.

danbi2990 · 2025-09-08T01:56:15Z

            .try_into()
            .expect("The number of resolved issues will not exceed i32::MAX");

+        let mut size_counts = HashMap::new();


Using fold would simplify the transformation.

Thanks for the suggestion. I’ve applied it.

Goder-0 · 2025-09-09T04:01:37Z

I have a suggestion regarding the implementation of the issue_stat function that could improve its performance and memory efficiency, especially when dealing with a large number of issues.

Current Approach

The current implementation processes the issues in multiple steps:

It first collects all matching issues into a Vec<Issue>.
It then iterates over this Vec to calculate open_issue_count.
It iterates over the Vec again to create a new Vec of resolved issues.
Finally, it iterates over the resolved_issues Vec to calculate the size distribution.

This approach involves multiple passes over the data and creates intermediate collections, which can be inefficient.

Proposed Improvement

We could refactor this to calculate all statistics in a single pass.

By applying the filter and then using a single .fold() operation, we can process the entire stream of issues just once. The accumulator for the fold could be a temporary struct or a tuple that holds all the necessary stats (open_issue_count, resolved_issue_count, size_counts).

Benefits

Performance: Avoids multiple iterations by processing the entire collection in one go.
Memory Efficiency: Prevents the allocation of intermediate Vec collections.

This is just a suggestion for a potential optimization, as the current code is functionally correct. Let me know what you think!

danbi2990 · 2025-09-09T05:19:40Z

I have a suggestion regarding the implementation of the issue_stat function that could improve its performance and memory efficiency, especially when dealing with a large number of issues.

I've thought about this as well. My idea can be illustrated as follows:

let mut stat1 = 0;
let mut stat2 = 0;

for issue in issues {
    // calculate and mutate in one pass
}

My concern is that this approach couples the calculation logic together, which reduces modularity. If we later change the calculation structure, it would be harder to refactor.

In my opinion, we should delay this discussion until the structure is more stable.

danbi2990 · 2025-09-09T07:41:22Z

+- Added `resolvedIssueSizeDistribution` field to `issueStat` query.
 - Added new statistics to GraphQL API `issueStat` query. A field
  `resolvedIssueCount` is added, indicating the number of resolved issues.
  Currently, an issue is defined to be resolved if and only if (1) it is


Could you move these entries (line 11-15) to line 23? I believe they should come after the creation of the issueStat query.

danbi2990 · 2025-09-09T07:42:27Z

+                    .map_or("None", |s| match s {
+                        "XS" | "S" | "M" | "L" | "XL" => s,
+                        _ => "None",
+                    });


Sorry for the confusion, but using HashMap<IssueSize, usize> with Hash derived seems more robust, since this match code is redundant given the From impl for IssueSize.

Could you revert to the previous code with Hash derived for IssueSize?
It will look like this:

let size_str = issue .project_items .nodes .iter() .find(|item| item.project_title == super::issue::TODO_LIST_PROJECT_TITLE) .and_then(|item| item.todo_size.as_deref()) .unwrap_or_default(); *acc.entry(IssueSize::from(size_str)).or_insert(0) += 1;

The reason I suggested using HashMap<&str, usize> is that I only considered the fixed set of strings: XS, S, M, L, XL and blank. However, if a new size such as "XXS" is introduced, it should first be filtered by the From implementation of IssueSize before being stored in the hashmap.

Please let me know if you have other ideas.

I've made a change: I used into instead of from. What do you think?

danbi2990 · 2025-09-09T08:22:34Z

+                count,
+            })
+            .collect();
+        resolved_issue_size_distribution.sort_by_key(|item| item.size);


Consider using sort_unstable_by_key, as it has some benefits over sort_by_key. See the "Rules on Sorting" page on Notion for details.

What do you think about using a BTreeMap instead of a HashMap here? Since the data size is usually small (around 5 items), the overhead would be negligible, and it would let us remove the extra sort_by_key step for a simpler implementation.

It seems reasonable. 👍

sophie-cluml · 2025-09-11T11:05:31Z

+- Added `resolvedIssueSizeDistribution` field to `issueStat` query, which
+  shows the distribution of sizes for resolved issues.


issueStat 도 Unreleased 섹션에서 새롭게 added 된 것이므로, resolvedIssueSizeDistribution 에 관한 것은 issueStat 의 addition에 관한 것에 포함되도록 해주세요.

수정했습니다.

Close #197

Goder-0 linked an issue Sep 5, 2025 that may be closed by this pull request

통계 구현: 인원, 기간, 저장소별 할당 & resolved 이슈의 Size 분포 #197

Closed

danbi2990 self-requested a review September 5, 2025 07:10

danbi2990 reviewed Sep 8, 2025

View reviewed changes

Goder-0 force-pushed the kai/stats-issue-size-distribution branch 2 times, most recently from b6b8f44 to cbeb8f2 Compare September 9, 2025 03:54

danbi2990 reviewed Sep 9, 2025

View reviewed changes

Goder-0 force-pushed the kai/stats-issue-size-distribution branch from cbeb8f2 to 3fe20bb Compare September 10, 2025 08:31

danbi2990 approved these changes Sep 11, 2025

View reviewed changes

danbi2990 requested a review from sophie-cluml September 11, 2025 04:19

Goder-0 force-pushed the kai/stats-issue-size-distribution branch from 3fe20bb to adc4d5d Compare September 11, 2025 07:44

sophie-cluml reviewed Sep 11, 2025

View reviewed changes

Add resolved_issue_size_distribution field

fe8ad8d

Close #197

Goder-0 force-pushed the kai/stats-issue-size-distribution branch from adc4d5d to fe8ad8d Compare September 12, 2025 04:55

sophie-cluml approved these changes Sep 12, 2025

View reviewed changes

Merge branch 'main' into kai/stats-issue-size-distribution

0ddcfec

danbi2990 merged commit a4d6075 into main Sep 15, 2025
12 checks passed

danbi2990 deleted the kai/stats-issue-size-distribution branch September 15, 2025 00:01

		item.project_title == super::issue::TODO_LIST_PROJECT_TITLE
		&& item.todo_status.as_deref() == Some(super::issue::TODO_LIST_STATUS_DONE)

		- Added `resolvedIssueSizeDistribution` field to `issueStat` query, which
		shows the distribution of sizes for resolved issues.

Uh oh!

Conversation

Goder-0 commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Graphql Response

Uh oh!

codecov Bot commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

danbi2990 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danbi2990 Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Goder-0 commented Sep 9, 2025

Current Approach

Proposed Improvement

Benefits

Uh oh!

danbi2990 commented Sep 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danbi2990 Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danbi2990 Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Goder-0 commented Sep 5, 2025 •

edited

Loading

codecov Bot commented Sep 5, 2025 •

edited

Loading

danbi2990 Sep 8, 2025 •

edited

Loading

danbi2990 Sep 9, 2025 •

edited

Loading

danbi2990 Sep 9, 2025 •

edited

Loading