Skip to content

Apply per-line discounts in the cart total - #23

Open
sergical wants to merge 1 commit into
mainfrom
demo-pr/cart-line-discount
Open

sergical wants to merge 1 commit into
mainfrom
demo-pr/cart-line-discount

Conversation

@sergical

@sergical sergical commented Oct 7, 2026

Copy link
Copy Markdown
Member

Demo pull request for the workshop. Do not merge.

Lets a cart item carry a discount in cents and subtracts it from the line before it is added to the total.

Created with Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e5c70d3. Configure here.

}
total += items[i].price * items[i].qty;
const line = items[i].price * items[i].qty;
total += line - (items[i].discountCents ?? 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Discount can make cart total negative

High Severity

discountCents is subtracted from the line in full, so cartTotal can return a negative amount when the discount exceeds price * qty or when quantity is zero. Zero-quantity items previously contributed nothing to the total.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit e5c70d3. Configure here.

}
total += items[i].price * items[i].qty;
const line = items[i].price * items[i].qty;
total += line - (items[i].discountCents ?? 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The discountCents value is not validated, which can lead to incorrect cart totals if it's negative or larger than the line item price.
Severity: HIGH

Suggested Fix

Add validation to ensure discountCents is not negative and does not exceed the line item's total price. Throw a RangeError if the value is invalid, similar to the existing validation for qty.

const line = items[i].price * items[i].qty;
if ((items[i].discountCents ?? 0) < 0) {
  throw new RangeError(`Negative discount for item ${i}`);
}
if ((items[i].discountCents ?? 0) > line) {
  throw new RangeError(`Discount exceeds line total for item ${i}`);
}
total += line - (items[i].discountCents ?? 0);
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: apps/pr-reviewer/fixtures/demo-pr/cart-total.ts#L10

Potential issue: The `cartTotal` function applies a discount from `discountCents`
without proper validation. This allows for two potential issues: 1) If `discountCents`
is a negative number, it will be added to the line item's total, incorrectly inflating
the final price. 2) If `discountCents` is greater than the line item's total price, it
will result in a negative value for that line item, which could lead to a negative
overall cart total, effectively paying a customer to take items.

Did we get this right? 👍 / 👎 to inform future reviews.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

Verdict: Needs changes — unit mismatch and missing validation around the new discountCents field should be fixed before merge.

Correctness

  1. Likely unit mismatch (price vs. cents) — const line = items[i].price * items[i].qty; total += line - (items[i].discountCents ?? 0); subtracts a cents-denominated value directly from a price * qty total whose unit is unspecified/unclear elsewhere in the codebase. If price is in dollars (or any non-cent unit), this silently produces wrong totals. This needs either a documented/enforced unit convention for price, or explicit conversion before subtracting.
  2. No validation on discountCents — qty is checked for negativity and throws RangeError, but discountCents has no equivalent guard. A negative discountCents would silently increase the total instead of discounting it.
  3. No cap on discount vs. line total — nothing prevents discountCents from exceeding the line's value, which could drive total negative without any error, unlike the existing fail-fast pattern used for qty.
  4. No handling of NaN/Infinity/non-integer cents — malformed discountCents values pass through silently, inconsistent with the defensive style already established for qty.

These are edge cases that could matter in production financial code (even in a fixture/demo file), and the existing codebase pattern (explicit RangeError on bad qty) suggests these checks were expected here too.

Style

  1. Inconsistent unit naming — discountCents has an explicit unit suffix while sibling field price does not, making it ambiguous whether they're meant to be in the same unit. Either rename both consistently (e.g. priceCents) or add a doc comment clarifying units on CartItem.
  2. line is a vague variable name — easily confused with "line number." Prefer lineTotal or lineSubtotal.
  3. Minor: the pre-existing C-style index loop could be modernized now that the loop body does more work, though this is optional/pre-existing and not blocking.
  4. CartItem is now exported without documentation; since it's part of the public surface, a short JSDoc noting units/invariants (e.g., discount should not exceed line total) would help future readers.

Tests

run_tests passed: 16 passed, 0 failed (2 suites ran). No regressions detected by the existing test suite, but this also suggests there is no test coverage yet for the new discountCents behavior (negative discount, discount > line total, unit correctness) — tests should be added for these before relying on the new field in production.

@sergical
sergical force-pushed the demo-pr/cart-line-discount branch from e5c70d3 to ed75ccc Compare October 7, 2026 03:31
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

Verdict: Request changes — the discount logic has unvalidated edge cases and a likely unit mismatch that should be addressed before merge.

Correctness

  1. No validation on discountCents. Unlike qty < 0, which throws a RangeError, a negative discountCents value is silently accepted and will increase the total instead of discounting it.
  2. No clamping/validation when discount exceeds the line total. line - discountCents can go negative for a single line item, and nothing prevents total itself from going negative. Decide whether this should be clamped to zero or rejected.
  3. Likely unit mismatch between price and discountCents. discountCents is explicitly denominated in cents, but price has no unit annotation and is combined with qty to form line without any indication it's also in cents. If price is in dollars (or another unit) while discountCents is in cents, the subtraction line - discountCents is wrong by orders of magnitude. This needs to be clarified/fixed or documented.
  4. No constraint (type-level or runtime) stops discountCents from being fractional, NaN, or Infinity, which would silently corrupt the result.

Style

  1. Naming inconsistency: price vs. discountCents — pick one convention (either both denote units explicitly, e.g. priceCents/discountCents, or neither does) so readers aren't left guessing about units.
  2. const line is vague; lineTotal/lineSubtotal would better convey that it's the pre-discount per-item amount.
  3. Repeated items[i] indexing (now 3x per iteration) could be simplified via destructuring (const { price, qty, discountCents = 0 } = items[i];) for readability.
  4. The discount-subtraction behavior (handling of negative/overshooting discounts) is undocumented, unlike the existing qty < 0 guard — a comment or explicit validation would help future maintainers.

Tests

run_tests passed: 16 passed, 0 failed (2 suites ran). No regressions detected by the existing test suite, but note the suite apparently doesn't cover the new discountCents edge cases (negative discount, discount > line total, unit mismatch) — consider adding tests for these before merging.

@linear-code

linear-code Bot commented Oct 12, 2026

Copy link
Copy Markdown

MAR-1676

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant