[WEB-3841] PDF bolus limit

[krystophv]

for WEB-3841, limits 3-hr binned bolus lists in the daily print view to 50 to avoid overflowing the allowed height

alongside tidepool-org/export#71 and tidepool-org/blip#1724

[Copilot]

Pull Request Overview

This PR implements a limit of 50 lines for 3-hour binned bolus lists in the daily print view to prevent PDF height overflow issues. The implementation includes proper truncation with ellipsis indication when the limit is exceeded.

• Adds a new method countBolusLinesWithLimit to calculate line usage and determine truncation needs
• Updates the bolus rendering logic to respect the 50-line limit and show an ellipsis when truncated
• Upgrades the pdfkit dependency from 0.13.0 to 0.15.0

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/modules/print/DailyPrintView.js	Implements the core bolus line counting and truncation logic with ellipsis support
test/modules/print/DailyPrintView.test.js	Adds comprehensive test coverage for the new truncation functionality
package.json	Updates version string and upgrades pdfkit dependency

[Copilot]

This condition check is duplicated from line 262 in the countBolusLinesWithLimit method. Consider extracting this logic into a helper method like isExtendedBolus(bolus) to avoid code duplication and improve maintainability.

[clintonium-119]

Looks good, except for a possible issue due to sorting.

[clintonium-119]

I see that you're sorting the boluses here prior to passing to countBolusLinesWithLimit, which makes sense.

However, you haven't done the same in when passing the 3h binned boluses in calculateDateChartHeight earlier.

I'm thinking it's possible that the earlier height is then calculated based on a different set of boluses that get rendered here.

i.e. perhaps the first 50 (of 60) when called within calculateDateChartHeight are normal boluses, but when you pass the sorted boluses in here, there are a few extended boluses that needed more lines than what was allotted for height.

Should we also pass sorted boluses in the earlier calls to countBolusLinesWithLimit?

[krystophv]

We can add the sort, for sure, but I can't think of a pathological case for a mismatch causing serious issues as is. If the first unsorted 50 of 60 happen to be 1 line boluses, we'd still end up counting at least up to the 50 line limit and returning the "count" there as 50. When it comes to this sorted set, we actually get the list of boluses to render also taking into account the 50 line limit. So the height count will always take into account the whole set up to the 50 line limit and the rendering will only render the first X number of boluses that will fit into a 50 line limit.

The only mismatch I can come up with would be potentially landing on a "count" returned for the height calculation of 49 instead of 50 by hitting an extended bolus and returning early on that count, but given that calculateDateChartHeight is just used for charts-per-page layout, I don't think that the 1 line difference could ever result in the wrong number of charts on a page (given proximity to the maximum).

For the sake of consistency, adding the sort is a good call though.

[clintonium-119]

Yeah, I get it. This whole PR is based on an unrealistic edge case, and the sort issue has a basically zero chance of happening.

You can put it for re-review and skip the sorting if you'd prefer.

[krystophv]

I added the sort (ecce2ff) 😄 , will put it up for re-review. And my reply was at least as much just me re-stating the logic to convince myself that I hadn't introduced a new edge case as it was trying to convince anyone else.

[clintonium-119]

LGTM 💯