Refactor linehaul calculation to subtract fuel surcharge before applying profit margin #2
Conversation
…ing profit margin - Move fuel surcharge calculation before linehaul rate calculation - Calculate linehaul rate by subtracting fuel from carrier rate first - Apply profit margin to linehaul rate instead of carrier rate - Update profit total calculation to be based on linehaul total - Maintain Math.max(0, ...) logic to prevent negative values Co-Authored-By: Anthony Fecarotta <anthony@linehaul.ai>
Original prompt from Anthony |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
- Change invoiceTotal calculation from avgTripTotal to allInTotal - This ensures profit margin is included in the final invoice total - Per-mile total now correctly includes profit in the calculation Co-Authored-By: Anthony Fecarotta <anthony@linehaul.ai>
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Request Changes
This PR refactors the freight rate calculation to apply profit margin after subtracting fuel surcharge, which reduces absolute profit dollars and changes core business logic. Manual testing is critical to verify the new calculations match business requirements.
📄 Documentation Diagram
This diagram documents the refactored freight rate calculation workflow with fuel surcharge subtracted before profit margin.
sequenceDiagram
participant U as User
participant C as Calculator
U->>C: Input: miles, carrierFlatRate, profitPercentage, fuelRate
C->>C: carrierRate = carrierFlatRate / miles
note over C: PR #35;2: Fuel calculation moved before profit application
C->>C: fuelTotal = miles * fuelRate
C->>C: fuelSurchargePerMile = fuelTotal / miles
C->>C: linehaulRate = max(0, carrierRate - fuelSurchargePerMile)
C->>C: linehaulTotal = miles * linehaulRate
C->>C: allInRate = linehaulRate * (1 + profitPercentage/100)
C->>C: profitTotal = linehaulTotal * profitPercentage/100
C->>C: allInTotal = miles * allInRate
C->>C: avgTripTotal = linehaulTotal + fuelTotal
C->>U: Output: various rates and totals
🌟 Strengths
- Preserved calculation dependency graph ensuring downstream functionality
- Maintained Math.max logic to prevent negative linehaul rates
| Priority | File | Category | Impact Summary | Anchors |
|---|---|---|---|---|
| P1 | calculator.js | Architecture | Changes profit calculation base, reducing absolute profit dollars | |
| P1 | calculator.js | Architecture | Profit now based on linehaul, not carrier rate | |
| P2 | calculator.js | Correctness and Business Logic | All-in rate definition changed, potential confusion | |
| P2 | calculator.js | Correctness and Business Logic | Avg trip total may undercharge customers | |
| P2 | calculator.js | Maintainability | Redundant fuel surcharge per mile calculation |
🔍 Notable Themes
- Business Logic Shift: The profit margin application has moved from gross carrier rate to net linehaul revenue, fundamentally altering financial outcomes.
- Calculation Semantics: Several output fields now have different meanings, which could lead to user confusion or incorrect billing if not addressed.
📈 Risk Diagram
This diagram illustrates the risk of reduced profit margins and potential billing inconsistencies due to the new calculation order.
sequenceDiagram
participant B as Broker
participant C as Calculator
B->>C: Input: carrierFlatRate, miles, fuelRate, profitPercentage
C->>C: carrierRate = carrierFlatRate / miles
C->>C: fuelTotal = miles * fuelRate
C->>C: linehaulRate = max(0, carrierRate - fuelSurchargePerMile)
note over C: R1(P1): Profit margin now on smaller base (linehaul vs carrier)
C->>C: profitTotal = linehaulTotal * profitPercentage / 100
note over C: R2(P1): Profit amount reduced for same percentage
C->>B: Output: allInTotal, profitTotal, avgTripTotal
note over B: Risk: Financial outcomes changed, potential underbilling
💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
calculator.js
Outdated
| // Calculate Linehaul Rate (Carrier Rate - Fuel Surcharge per mile) | ||
| const linehaulRate = Math.max(0, carrierRate - fuelSurchargePerMile); | ||
| linehaulRateInput.value = linehaulRate.toFixed(2); | ||
|
|
||
| // Calculate Linehaul Total (miles * linehaul rate) | ||
| const linehaulTotal = miles * linehaulRate; | ||
| linehaulTotalInput.value = linehaulTotal.toFixed(2); | ||
|
|
||
| // Calculate All-In Rate (linehaul rate * gross profit percentage) | ||
| const grossProfitMultiplier = 1 + (profitPercentage / 100); |
There was a problem hiding this comment.
P1 | Confidence: High
The calculation order change fundamentally alters the business logic by applying profit margin to linehaul revenue only (after fuel costs) rather than total carrier revenue. This creates a direct financial impact: profit margins now apply to a smaller base amount, reducing absolute profit dollars for the same percentage margin. The change is architecturally significant as it redefines core financial calculations used throughout the freight quoting system. The Math.max(0, ...) protection prevents negative linehaul rates but may mask business logic errors where fuel costs exceed carrier rates.
calculator.js
Outdated
| // Calculate Gross Profit Total (linehaul total * profit percentage / 100) | ||
| const profitTotal = linehaulTotal * (profitPercentage / 100); | ||
| profitTotalInput.value = profitTotal.toFixed(2); |
There was a problem hiding this comment.
P1 | Confidence: High
The profit calculation now uses linehaulTotal instead of carrierFlatRate, creating a direct financial impact where profit amounts will be lower for the same percentage margin. This changes the fundamental profit model from markup on total carrier cost to markup on net linehaul revenue. The related_context shows this profitTotal feeds into downstream calculations including invoice totals and per-mile rates, creating a ripple effect throughout the financial reporting.
| const linehaulTotal = miles * linehaulRate; | ||
| linehaulTotalInput.value = linehaulTotal.toFixed(2); | ||
|
|
||
| // Calculate average trip total (linehaul total + fuel total) |
There was a problem hiding this comment.
P2 | Confidence: High
The avgTripTotal calculation now sums linehaulTotal (which excludes profit) with fuelTotal, whereas previously it summed linehaulTotal (which included profit) with fuelTotal. This creates an inconsistency where avgTripTotal no longer represents the total customer charge but rather the carrier cost plus fuel. The related_context shows this value feeds into invoiceTotal, potentially undercharging customers if this represents the final invoice amount.
| // Calculate average trip total (linehaul total + fuel total) | |
| // Calculate average trip total (linehaul total + fuel total + profit) | |
| const avgTripTotal = linehaulTotal + fuelTotal + profitTotal; | |
| if (!allInTotalInput) { | |
| avgTripTotalInput.value = avgTripTotal.toFixed(2); | |
| } |
calculator.js
Outdated
| // Calculate Fuel Surcharge total (miles * fuel rate) | ||
| const fuelTotal = miles * fuelRate; | ||
| fuelTotalInput.value = fuelTotal.toFixed(2); | ||
|
|
||
| // Calculate All-In Rate (carrier rate * gross profit percentage) | ||
| // Calculate Fuel Surcharge per mile |
There was a problem hiding this comment.
P2 | Confidence: High
The fuel surcharge calculation now occurs earlier in the flow, improving logical sequencing. However, fuelSurchargePerMile is calculated by dividing fuelTotal by miles, which simplifies to fuelRate. This creates redundant calculations that could be simplified for better maintainability and performance.
| // Calculate Fuel Surcharge total (miles * fuel rate) | |
| const fuelTotal = miles * fuelRate; | |
| fuelTotalInput.value = fuelTotal.toFixed(2); | |
| // Calculate All-In Rate (carrier rate * gross profit percentage) | |
| // Calculate Fuel Surcharge per mile | |
| // Calculate Fuel Surcharge total (miles * fuel rate) | |
| const fuelTotal = miles * fuelRate; | |
| fuelTotalInput.value = fuelTotal.toFixed(2); | |
| // Fuel Surcharge per mile equals fuelRate | |
| const fuelSurchargePerMile = fuelRate; |
calculator.js
Outdated
| // Calculate All-In Total (miles * all-in rate) | ||
| const allInTotal = miles * allInRate; | ||
| if (allInTotalInput) { | ||
| allInTotalInput.value = allInTotal.toFixed(2); | ||
| } else { | ||
| avgTripTotalInput.value = allInTotal.toFixed(2); | ||
| } |
There was a problem hiding this comment.
P2 | Confidence: Medium
The allInTotal calculation remains unchanged but now uses a different allInRate definition. This creates potential confusion as the same field name represents different underlying calculations. The related_context shows this value is used in conditional UI logic (allInTotalInput vs avgTripTotalInput), suggesting this field serves multiple purposes. The calculation dependency chain is preserved, but the semantic meaning of "all-in" has shifted from carrier-rate-plus-profit to linehaul-plus-profit.
…ubtract fuel BREAKING CHANGE: Reverses previous refactor to match correct business logic The correct calculation order is: 1. Apply profit margin to carrier rate to get ALL-IN RATE - ALL-IN TOTAL = CARRIER FLAT × (1 + profit%) - ALL-IN RATE = ALL-IN TOTAL ÷ miles 2. Subtract fuel surcharge from all-in rate to get LINEHAUL RATE - LINEHAUL RATE = ALL-IN RATE - FUEL RATE - LINEHAUL TOTAL = LINEHAUL RATE × miles This ensures: - ALL-IN RATE = CARRIER RATE + GROSS PROFIT % (as specified) - Profit is applied to carrier cost BEFORE fuel separation - LINEHAUL RATE is calculated AFTER profit is applied (not inflated) Example verification (1000 miles, $1000 carrier, 10% profit): - Carrier Rate: $1.00/mile - Profit: $100 (10% of $1000) - All-In Total: $1100 ✓ - All-In Rate: $1.10/mile ✓ - Fuel: $0.26/mile - Linehaul Rate: $0.84/mile ($1.10 - $0.26) ✓ Co-Authored-By: Anthony Fecarotta <anthony@linehaul.ai>
Correct Linehaul Rate Calculation Order
Summary
This PR fixes the calculation order in the freight rate calculator to match the correct business logic:
Correct Order (implemented):
Key Formula:
ALL-IN RATE = CARRIER RATE + GROSS PROFIT %Example: $1,000 carrier flat + 10% profit = $1,100 all-in total ✓
Changes Made:
allInTotaldirectly fromcarrierFlatRate × (1 + profit%)to avoid rounding driftallInRateby dividingallInTotalby mileslinehaulRate = allInRate - fuelRateallInTotal(includes profit) instead ofavgTripTotal(excludes profit)fuelSurchargePerMilevariable (just usefuelRatedirectly)Review & Testing Checklist for Human
Recommended Test Plan
Notes
Testing Done:
sidepanel.htmldirectly in browserImportant: This changes the pricing calculation significantly. The linehaul rate is now calculated AFTER profit is applied (not before), which means profit is applied to the full carrier cost before fuel is separated out.
Session Info: