Fix library_hold_ratio integer division in inventory report (PP-4552)

[dbernstein]

Description

library_hold_ratio divided a bigint (from count()) by an integer column, so PostgreSQL truncated every ratio below 1.0 to zero. The numerator is now cast to numeric and the result rounded to two places.

A title with 31 holds against 61 owned copies reported 0; it now reports 0.51.

Note: this PR was originally larger and has been re-split. It is now the hold-ratio fix only; all loan-count changes moved to #3763. The branch name is left over from the original split and no longer describes the contents.

Motivation and Context

PP-4552. Library admins reported inventory activity report figures that disagreed with the Collection Manager.

How Has This Been Tested?

New test_inventory_activity_report_hold_ratio covers a fractional ratio (31/61), a whole-number ratio, zero holds, and the -1 no-owned-copies sentinel. Confirmed to fail against the pre-fix query, reporting 0.0 instead of 0.51.

The existing suite missed this because its only non-zero case was 3 holds / 1 license, where truncation is a no-op.

19 tests pass. mypy and pre-commit clean.

Checklist

• I have updated the documentation accordingly.
• All new and existing tests passed.

🤖 Generated with Claude Code

[claude]

Claude finished @dbernstein's task in 1m 23s —— View job

---

Summary

This looks good to merge. I found nothing that needs changing. The fix is small and correct: casting the numerator to Numeric makes PostgreSQL resolve the whole CASE to numeric, so the -1 sentinel still comes out as -1. That means the existing int(no_licenses_owned_row["library_hold_ratio"]) == -1 assertion in test_generate_report keeps passing. The values now reach the writers as Decimal. csv.QUOTE_NONNUMERIC treats Decimal as a number and leaves it unquoted, and openpyxl writes it as a number cell, so the column stays numeric in both the CSV and XLSX outputs. The only visible difference is formatting: whole ratios now print as 3.00 / 0.00 instead of 3 / 0. This division was the only one in the module, so no other column has the same bug. All the earlier review comments are addressed:

• The docstring now says truncation happens at any magnitude.
• There is now a fraction-below-1 case (31/61) and a fraction-above-1 case (7/2), plus the whole-number case.
• The nested test helpers are annotated.

Because the test compares with pytest.approx at its default tolerance, it would also fail if the round(…, 2) were dropped (0.508… ≠ 0.51).

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Fixes integer division bug in hold ratio calculation.

The hold-ratio change appears safe to merge, with no outstanding findings in this PR.

Summary

This PR casts the hold count to a numeric value before calculating library_hold_ratio, preserving fractional results, and adds tests for fractional, whole-number, zero-hold, and no-owned-copy cases.

_{Reviews (8) · Last reviewed commit: "Correct hold ratio truncation wording an..."}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.71%. Comparing base (54de833) to head (028f078).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3762      +/-   ##
==========================================
- Coverage   93.71%   93.71%   -0.01%     
==========================================
  Files         510      510              
  Lines       46515    46518       +3     
  Branches     6314     6314              
==========================================
  Hits        43593    43593              
- Misses       1888     1889       +1     
- Partials     1034     1036       +2     

☔ View full report in Codecov by Harness.
📢 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.

[dbernstein]

Both review findings confirmed and addressed — this PR has been re-split.

I reproduced each finding against a live database rather than reasoning from the code:

• ODL pools — a license with concurrency 1 and 1 checkout left goes to owned 0 / available 0 / reserved 0 the moment it is borrowed, so the subtraction reported 0 while a patron held the loan. With concurrency 5 and 3 checkouts left, 2 copies out reported 0 instead of 2. The formula is only correct while checkouts_left - k >= terms_concurrency.
• Unlimited pools — 3 active loans reported 0, since all four counters are pinned to zero at import.

Both produced rows where library_active_loan_count exceeded shared_active_loan_count, which is self-evidently wrong.

Since the two findings share one root cause — deriving loans from availability counters — the fix is to count Loan rows instead, per @claude's suggestion. That is exact for all three pool types and guarantees library <= shared.

Re-split along the conceptual seam so each PR is focused:

• This PR is now the library_hold_ratio integer-division fix only. No semantics, no open questions.
• Count inventory report loans from Loan rows (PP-4552) #3763 carries all loan-counting work: both laterals folded into one helper with a shared definition of "active", the Loan-row count, and the expired-loan filter.

The branch name here is left over from the original split and no longer matches the contents; renaming would have meant closing this PR, so I left it.

New test asserts 31/61 = 0.51 and was confirmed to fail against the pre-fix query.

[tdilauro]

This looks mostly good, but has some misleading test cases and comments. Approving now, but please resolve the comments before merging.