Skip to content

fix: Overall capacity summary is inconsistent with owner workloads #282 #307

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.

Sign up for GitHub

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

Open
YuliaMittova wants to merge 1 commit into
base: toloka-agent/159-auto
Choose a base branch
from
Open

fix: Overall capacity summary is inconsistent with owner workloads #282 #307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 15 additions & 33 deletions src/main/java/com/sivalabs/ft/features/domain/PlanningAnalyticsService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import com.sivalabs.ft.features.api.models.PlanningTrendsResponse;
import com.sivalabs.ft.features.domain.entities.Feature;
import com.sivalabs.ft.features.domain.entities.Release;
import com.sivalabs.ft.features.domain.models.FeaturePlanningStatus;
import com.sivalabs.ft.features.domain.models.ReleaseStatus;
import java.math.BigDecimal;
import java.math.RoundingMode;
Expand All @@ -22,6 +21,8 @@
@Service
public class PlanningAnalyticsService {

private static final int DEFAULT_CAPACITY_PER_PERSON = 10;

private final ReleaseRepository releaseRepository;
private final FeatureRepository featureRepository;
private final ApplicationProperties applicationProperties;
Expand Down Expand Up @@ -215,42 +216,23 @@ private PlanningTrendsResponse.PlanningAccuracyTrend calculatePlanningAccuracyTr
}

private CapacityPlanningResponse.OverallCapacity calculateOverallCapacity(List<Feature> features) {
Set<String> uniqueOwners = features.stream()
.map(Feature::getFeatureOwner)
.filter(Objects::nonNull)
.collect(Collectors.toSet());

int totalResources = uniqueOwners.size();

// Simplified capacity calculation
double totalUtilization = features.stream()
Map<String, List<Feature>> featuresByOwner = features.stream()
.filter(f -> f.getFeatureOwner() != null)
.collect(Collectors.groupingBy(Feature::getFeatureOwner))
.values()
.stream()
.mapToDouble(ownerFeatures -> {
int assigned = ownerFeatures.size();
int completed = (int) ownerFeatures.stream()
.filter(f -> f.getPlanningStatus() == FeaturePlanningStatus.DONE)
.count();
return assigned > 0 ? (double) completed / assigned * 100 : 0;
})
.average()
.orElse(0.0);
.collect(Collectors.groupingBy(Feature::getFeatureOwner));

double utilizationRate = round(totalUtilization, 1);
int totalResources = featuresByOwner.size();
if (totalResources == 0) {
return new CapacityPlanningResponse.OverallCapacity(0, 0.0, 100.0, 0);
}
int totalAmountOfFeatures =
featuresByOwner.values().stream().mapToInt(List::size).sum();
double totalUtilization = (double) totalAmountOfFeatures / (DEFAULT_CAPACITY_PER_PERSON * totalResources);
Copy link
Copy Markdown

@ElizavetaP ElizavetaP Apr 1, 2026

Choose a reason for hiding this comment

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

This bugfix introduces a small edge-case regression: when there are no owners, the calculation becomes 0 / 0, which produces NaN and then fails during rounding.
It is not the main issue, but it would be better to address it as well.
A simple way to fix this would be to add an explicit zero-owner check, for example as in the golden implementation.

Copy link
Copy Markdown

@innokenty innokenty Apr 1, 2026

Choose a reason for hiding this comment

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

I am not sure if maybe we never end here, because there's a safeguard somewhere before, but please add a test – that's a good use case!

Copy link
Copy Markdown
Author

@YuliaMittova YuliaMittova Apr 7, 2026

Choose a reason for hiding this comment

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

@ElizavetaP I tried to apply golden implementation from here and I see that it calculates totalUtilization with a mistake:

        double totalUtilization = features.stream()
                .filter(f -> f.getFeatureOwner() != null)
                .collect(Collectors.groupingBy(Feature::getFeatureOwner))
                .values()
                .stream()
                .mapToDouble(ownerFeatures -> {
                    int assigned = ownerFeatures.size();
                    int completed = (int) ownerFeatures.stream()
                            .filter(f -> f.getPlanningStatus() == FeaturePlanningStatus.DONE)
                            .count();
                    return assigned > 0 ? (double) completed / assigned * 100 : 0;
                })
                .average()
                .orElse(0.0);

For example, if one person has 10 tasks in progress and another one has 1 task in progress then totalUtilization will be 0 because none of the tasks are completed. Can you tell what is expected here? Is this logic intended?

Copy link
Copy Markdown

@ElizavetaP ElizavetaP Apr 8, 2026

Choose a reason for hiding this comment

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

@YuliaMittova I only meant the NaN edge case.

If there are no owners, this becomes 0 / 0, which produces NaN. My comment was only about guarding that case, not about copying the whole golden calculation logic.

Copy link
Copy Markdown

@innokenty innokenty Apr 8, 2026

Choose a reason for hiding this comment

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

That's true, I'm already fixing this.

double utilizationRate = round(totalUtilization * 100.0, 1);
double availableCapacity = round(100.0 - utilizationRate, 1);

int overallocatedResources = (int) features.stream()
.filter(f -> f.getFeatureOwner() != null)
.collect(Collectors.groupingBy(Feature::getFeatureOwner))
.entrySet()
.stream()
.filter(entry -> {
List<Feature> ownerFeatures = entry.getValue();
int assigned = ownerFeatures.size();
return assigned > 10; // Simplified threshold
})
int overallocatedResources = (int) featuresByOwner.values().stream()
.filter(ownerFeatures -> ownerFeatures.size() > 10) // Simplified threshold
.count();

return new CapacityPlanningResponse.OverallCapacity(
Expand All @@ -268,7 +250,7 @@ private List<CapacityPlanningResponse.WorkloadByOwner> calculateWorkloadByOwner(
List<Feature> ownerFeatures = entry.getValue();

int currentWorkload = ownerFeatures.size();
int capacity = 10; // Simplified capacity
int capacity = DEFAULT_CAPACITY_PER_PERSON;
double utilizationRate = round((double) currentWorkload / capacity * 100, 1);
int futureCommitments = currentWorkload + 2; // Simplified

Expand Down
Loading
Loading