Skip to content

Commit

Permalink
Only consider genpop appeals when attempting to evenly distribute cas…
Browse files Browse the repository at this point in the history
…es to judges (#15328)

Resolves discrepancy found here: https://dsva.slack.com/archives/CN3FQR4A1/p1601297485008600

### Description
Our priority push job runs in 2 phases.
First, we distribute all cases that are tied to a judge through previous decision or a held hearing.
Second, we pull a count of all remaining priority cases ready to be distributed and calculate how to distribute them evenly across judges.
However, when we pull the count of remaining cases, we were previously including cases that are tied to judges but could not be distributed. This could happen for a number of reasons (judge case is tied to no longer works for the board, is not a user in caseflow, is an AVLJ, etc). The issue is that when we try evenly distribute remaining cases, we cannot distribute cases tied to judges to another judge.
This let to us only having 60 cases to distribute, but our calculations were done assuming there would be 350+ cases to distribute.

This PR ensures the calculations done on `ready_priority_appeals_count` do not consider cases we cannot distribute (cases tied to another judge)

### Acceptance Criteria
- [ ] When calculating priority target for the acd priority push job, only consider genpop cases

### Testing Plan
1. in prod
```ruby
PushPriorityAppealsToJudgesJob.new.ready_priority_appeals_count
=> 346
# Or some other 300+ number

# what should this be instead?
query = <<-SQL
  #{VACOLS::CaseDocket::SELECT_PRIORITY_APPEALS}
  where VLJ is null
SQL

VACOLS::CaseDocket.connection.exec_query(query).to_hash.count
=> 34
HearingRequestDistributionQuery.new(base_relation: HearingRequestDocket.new.appeals(ready: true, priority: true), genpop: "only_genpop").call.count
=> 0
DirectReviewDocket.new.count(ready: true, priority: true)
=> 0
EvidenceSubmissionDocket.new.count(ready: true, priority: true)
=> 0
# The sum of these numbers should be MUCH lower than the one returned from ready_priority_appeals_count
```
  • Loading branch information
hschallhorn authored Sep 29, 2020
1 parent 0cf9233 commit aec6a6a
Show file tree
Hide file tree
Showing 10 changed files with 301 additions and 183 deletions.
10 changes: 5 additions & 5 deletions app/jobs/push_priority_appeals_to_judges_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def eligible_judge_target_distributions_with_leftovers
# Because we cannot distribute fractional cases, there can be cases leftover after taking the priority target
# into account. This number will always be less than the number of judges that need distribution because division
def leftover_cases_count
ready_priority_appeals_count - target_distributions_for_eligible_judges.values.sum
ready_genpop_priority_appeals_count - target_distributions_for_eligible_judges.values.sum
end

# Calculate the number of cases a judge should receive based on the priority target. Don't toss out judges with 0 as
Expand All @@ -105,14 +105,14 @@ def target_distributions_for_eligible_judges
def priority_target
@priority_target ||= begin
distribution_counts = priority_distributions_this_month_for_eligible_judges.values
target = (distribution_counts.sum + ready_priority_appeals_count) / distribution_counts.count
target = (distribution_counts.sum + ready_genpop_priority_appeals_count) / distribution_counts.count

# If there are any judges that have previous distributions that are MORE than the currently calculated priority
# target, no target will be large enough to get all other judges up to their number of cases. Remove them from
# consideration and recalculate the target for all other judges.
while distribution_counts.any? { |distribution_count| distribution_count > target }
distribution_counts = distribution_counts.reject { |distribution_count| distribution_count > target }
target = (distribution_counts.sum + ready_priority_appeals_count) / distribution_counts.count
target = (distribution_counts.sum + ready_genpop_priority_appeals_count) / distribution_counts.count
end

target
Expand All @@ -123,8 +123,8 @@ def docket_coordinator
@docket_coordinator ||= DocketCoordinator.new
end

def ready_priority_appeals_count
@ready_priority_appeals_count ||= docket_coordinator.priority_count
def ready_genpop_priority_appeals_count
@ready_genpop_priority_appeals_count ||= docket_coordinator.genpop_priority_count
end

# Number of priority distributions every eligible judge has received in the last month
Expand Down
6 changes: 3 additions & 3 deletions app/models/concerns/proportion_hash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

module ProportionHash
def normalize!(to: 1.0)
total = values.reduce(0, :+)
total = values.sum
transform_values! { |proportion| proportion * (to / total) }
end

Expand All @@ -24,7 +24,7 @@ def all_zero?

def add_fixed_proportions!(fixed)
except!(*fixed.keys)
.normalize!(to: 1.0 - fixed.values.reduce(0, :+))
.normalize!(to: 1.0 - fixed.values.sum)
.merge!(fixed)
end

Expand Down Expand Up @@ -52,7 +52,7 @@ def add_fixed_proportions!(fixed)

def stochastic_allocation(num)
result = transform_values { |proportion| (num * proportion).floor }
rem = num - result.values.reduce(0, :+)
rem = num - result.values.sum

return result if rem == 0

Expand Down
5 changes: 5 additions & 0 deletions app/models/docket.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ def count(priority: nil, ready: nil)
appeals(priority: priority, ready: ready).ids.size
end

# By default all cases are considered genpop. This can be overridden for specific dockets
def genpop_priority_count
count(priority: true, ready: true)
end

def weight
count
end
Expand Down
8 changes: 6 additions & 2 deletions app/models/docket_coordinator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,11 @@ def priority_count
@priority_count ||= dockets
.values
.map { |docket| docket.count(priority: true, ready: true) }
.reduce(0, :+)
.sum
end

def genpop_priority_count
@genpop_priority_count ||= dockets.values.map(&:genpop_priority_count).sum
end

def direct_review_due_count
Expand Down Expand Up @@ -140,6 +144,6 @@ def docket_margin_net_of_priority
end

def nonpriority_decisions_per_year
@nonpriority_decisions_per_year ||= [LegacyAppeal, Docket].map(&:nonpriority_decisions_per_year).reduce(0, :+)
@nonpriority_decisions_per_year ||= [LegacyAppeal, Docket].map(&:nonpriority_decisions_per_year).sum
end
end
13 changes: 10 additions & 3 deletions app/models/dockets/hearing_request_docket.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,21 @@ def docket_type
Constants.AMA_DOCKETS.hearing
end

def age_of_n_oldest_genpop_priority_appeals(num)
relation = appeals(priority: true, ready: true).limit(num)
def ready_priority_appeals
appeals(priority: true, ready: true)
end

def age_of_n_oldest_genpop_priority_appeals(num)
HearingRequestDistributionQuery.new(
base_relation: relation, genpop: "only_genpop"
base_relation: ready_priority_appeals.limit(num), genpop: "only_genpop"
).call.map(&:ready_for_distribution_at)
end

# Hearing cases distinguish genpop from cases tied to a judge
def genpop_priority_count
HearingRequestDistributionQuery.new(base_relation: ready_priority_appeals, genpop: "only_genpop").call.count
end

def distribute_appeals(distribution, priority: false, genpop: "any", limit: 1)
base_relation = appeals(priority: priority, ready: true).limit(limit)

Expand Down
6 changes: 5 additions & 1 deletion app/models/dockets/legacy_docket.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,16 @@ def count(priority: nil, ready: nil)
end
# rubocop:enable Metrics/CyclomaticComplexity

def genpop_priority_count
LegacyAppeal.repository.genpop_priority_count
end

def weight
count(priority: false) + nod_count * NOD_ADJUSTMENT
end

def ready_priority_appeal_ids
VACOLS::CaseDocket.priority_ready_appeal_vacols_ids
LegacyAppeal.repository.priority_ready_appeal_vacols_ids
end

def oldest_priority_appeal_days_waiting
Expand Down
9 changes: 9 additions & 0 deletions app/models/vacols/case_docket.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,15 @@ def self.counts_by_priority_and_readiness
end
# rubocop:enable Metrics/MethodLength

def self.genpop_priority_count
query = <<-SQL
#{SELECT_PRIORITY_APPEALS}
where VLJ is null
SQL

connection.exec_query(query).to_hash.count
end

def self.nod_count
where("BFMPRO = 'ADV' and BFD19 is null").count
end
Expand Down
16 changes: 16 additions & 0 deletions app/repositories/appeal_repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,22 @@ def docket_counts_by_priority_and_readiness
end
end

def genpop_priority_count
MetricsService.record("VACOLS: genpop_priority_count",
name: "genpop_priority_count",
service: :vacols) do
VACOLS::CaseDocket.genpop_priority_count
end
end

def priority_ready_appeal_vacols_ids
MetricsService.record("VACOLS: priority_ready_appeal_vacols_ids",
name: "priority_ready_appeal_vacols_ids",
service: :vacols) do
VACOLS::CaseDocket.priority_ready_appeal_vacols_ids
end
end

def nod_count
MetricsService.record("VACOLS: nod_count",
name: "nod_count",
Expand Down
12 changes: 6 additions & 6 deletions spec/jobs/push_priority_appeals_to_judges_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ def to_judge_hash(arr)
job.instance_variable_set(:@genpop_distributions, distributed_cases)
allow_any_instance_of(PushPriorityAppealsToJudgesJob)
.to receive(:priority_distributions_this_month_for_eligible_judges).and_return(previous_distributions)
allow_any_instance_of(DocketCoordinator).to receive(:priority_count).and_return(20)
allow_any_instance_of(DocketCoordinator).to receive(:genpop_priority_count).and_return(20)
end

it "returns ids and age of ready priority appeals not distributed" do
Expand Down Expand Up @@ -356,7 +356,7 @@ def to_judge_hash(arr)
allow_any_instance_of(PushPriorityAppealsToJudgesJob)
.to receive(:priority_distributions_this_month_for_eligible_judges)
.and_return(to_judge_hash(distribution_counts))
allow_any_instance_of(DocketCoordinator).to receive(:priority_count).and_return(priority_count)
allow_any_instance_of(DocketCoordinator).to receive(:genpop_priority_count).and_return(priority_count)
end

subject { PushPriorityAppealsToJudgesJob.new.eligible_judge_target_distributions_with_leftovers }
Expand Down Expand Up @@ -465,7 +465,7 @@ def to_judge_hash(arr)
allow_any_instance_of(PushPriorityAppealsToJudgesJob)
.to receive(:priority_distributions_this_month_for_eligible_judges).and_return(@distribution_counts)
allow_any_instance_of(PushPriorityAppealsToJudgesJob)
.to receive(:ready_priority_appeals_count).and_return(priority_count)
.to receive(:ready_genpop_priority_appeals_count).and_return(priority_count)
end

it "evens out over multiple calls" do
Expand All @@ -486,7 +486,7 @@ def to_judge_hash(arr)
before do
allow_any_instance_of(PushPriorityAppealsToJudgesJob)
.to receive(:target_distributions_for_eligible_judges).and_return(target_distributions)
allow_any_instance_of(DocketCoordinator).to receive(:priority_count).and_return(priority_count)
allow_any_instance_of(DocketCoordinator).to receive(:genpop_priority_count).and_return(priority_count)
end

subject { PushPriorityAppealsToJudgesJob.new.leftover_cases_count }
Expand Down Expand Up @@ -531,7 +531,7 @@ def to_judge_hash(arr)
allow_any_instance_of(PushPriorityAppealsToJudgesJob)
.to receive(:priority_distributions_this_month_for_eligible_judges)
.and_return(to_judge_hash(distribution_counts))
allow_any_instance_of(DocketCoordinator).to receive(:priority_count).and_return(priority_count)
allow_any_instance_of(DocketCoordinator).to receive(:genpop_priority_count).and_return(priority_count)
end

subject { PushPriorityAppealsToJudgesJob.new.target_distributions_for_eligible_judges }
Expand Down Expand Up @@ -649,7 +649,7 @@ def to_judge_hash(arr)
allow_any_instance_of(PushPriorityAppealsToJudgesJob)
.to receive(:priority_distributions_this_month_for_eligible_judges)
.and_return(to_judge_hash(distribution_counts))
allow_any_instance_of(DocketCoordinator).to receive(:priority_count).and_return(priority_count)
allow_any_instance_of(DocketCoordinator).to receive(:genpop_priority_count).and_return(priority_count)
end

subject { PushPriorityAppealsToJudgesJob.new.priority_target }
Expand Down
Loading

0 comments on commit aec6a6a

Please sign in to comment.