fix(audit): show the price a line sold for, not its list price

[mforce]

TL;DR

The audit Details cell printed each sales line's list price as though it were the price the line sold for. AuditDetails read unitPriceMinorUnits, used it only as a null guard, then discarded it whenever a list price existed — so every SalesOrder.AddItem row rendered at list $<list>, whatever it actually sold for.

One branch in one component, plus tests. No API, payload, migration or i18n-key change.

The defect, on fixture order SO-1318252C

Line	List	Sold	Sales screen	Audit before	Audit after
Sim Cracked Eggs	$0.18	$0.15	Below list, 16.7%	at list $0.18 ❌	at $0.15 (list $0.18)
Sim Dirty Eggs	$0.22	$0.30	Above list	at list $0.22 ❌	at $0.30 (list $0.22)
Sim Small Eggs	$0.30	$0.30	at list	at list $0.30 ✅	at list $0.30
Sim Large Eggs	$0.45	$0.20	Below list, 55.6%	at list $0.45 ❌	at $0.20 (list $0.45)

Three of four were wrong. The worst read as a line selling at $0.45 that sold at $0.20.

Introduced by d26d389 (#749, for #745) — the commit that built the Details column. It shipped this way; it did not drift. It survived review and every test because at list $0.45 is a plausible, correctly formatted number, and nothing on screen contradicted it until #721 began printing Discount reason: Volume directly beneath four rows all claiming to have sold at list.

Rendering contract

Case	Renders	Changed?
no list price (list === null)	at $0.15	no
sold at list (unit === list)	at list $0.30	no
sold below or above list	at $0.20 (list $0.45)	the fix

An off-list line gets no marker word in either direction. #719's open question 4 deliberately treats a markup as not a discount, so the cell states both facts and lets the reader see the direction rather than editorialising. That reasoning is a comment in the code, not just here.

No new translation strings. detailsAtPrice, detailsAtList and detailsListParen already existed in en, es and tl (3/3 each); detailsListParen is the same key the UpdateItem branch already renders in the same position, so per #688 each locale keeps its own existing wording for list price rather than a fresh translation.

Reviewer entry points

• web/src/routes/AuditPage.tsx — the whole behaviour change is the SalesOrder.AddItem ternary, ~6 lines. Everything else in the diff is the comment above it.
• web/src/routes/AuditPage.test.tsx — the four new cases.

One deletion to look at deliberately

The previous test, "renders an AddItem payload as the artboard's summary", asserted /\$0\.45/ against a payload with unitPriceMinorUnits: 40, listUnitPriceMinorUnits: 45. It pinned the defect, not the intended behaviour — it passed precisely because the wrong number was on screen. It is replaced rather than weakened, by four tests asserting the cell's literal full text (toBe, on the cell's textContent, which is what pins the order of the two prices — a regex on one number is what let this ship).

Mutation evidence

Every new test was watched fail before being claimed. Three mutations, each reverted; git grep -n -e MUTANT -e 'DEBUG-' -- src tests web returns nothing.

Mutation	Tests red	Observed
M1 restore the shipped defect (money(list) in the off-list branch)	below-list, above-list	expected 'Large Eggs ×240 at list $0.45' to be 'Large Eggs ×240 at $0.20 (list $0.45)'
M2 drop the unit === list collapse	at-list	expected 'Large Eggs ×240 at $0.30 (list $0.30)' to be 'Large Eggs ×240 at list $0.30'
M3 stop list === null short-circuiting	no-list-price	expected 'Large Eggs ×240 at $0.15 (list $0.00)' to be 'Large Eggs ×240 at $0.15' — M3 reproduces #720's zero-price failure mode exactly

All four tests are individually load-bearing: no mutation reddened a test that was not aimed at it.

Gates

cd web && npm run typecheck   # clean
cd web && npx vitest run      # 122 files, 2816 tests, all passing

Web-only — nothing outside web/ changes, so no .NET build and no integration suite.

Sibling check

grep -rn listUnitPriceMinorUnits web/src finds one other renderer, SalesPage.tsx, which reads the sale and list prices separately and is the screen the issue confirms was already correct. No sibling defect; no coverage added there.

Risk

Low. Read-only rendering of a payload field already present on every row, in one component, behind four literal-text tests. No backfill concern: the change is presentational, so historical rows render correctly the moment this ships. The at-list and no-list-price paths are byte-identical to before and pinned as regressions.

Closes #758

Summary by CodeRabbit

• Bug Fixes
  • Audit details now consistently display the sold unit price for added sales order items.
  • When a list price is available, matching prices are labeled “at list,” while differing prices show both the sold and list prices without classifying the difference as a discount or markup.
  • Items without a list price continue to display their existing details.

[coderabbitai]

Warning

Review limit reached

Next included review available in 27 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d16cebc4-7bef-4c95-8f93-5ba04185d0bf

📥 Commits

Reviewing files that changed from the base of the PR and between c7864be and 8c4ea41.

📒 Files selected for processing (2)

• web/src/routes/AuditPage.test.tsx
• web/src/routes/AuditPage.tsx

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: ac1c7e6b-ba6b-4487-a13b-e9e6c6ed88e8

📥 Commits

Reviewing files that changed from the base of the PR and between c7864be and 8c4ea41.

📒 Files selected for processing (2)

• web/src/routes/AuditPage.test.tsx
• web/src/routes/AuditPage.tsx

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

SalesOrder.AddItem audit details now display the sold unit price. They also display the list price when it differs, retain “at list” for matching prices, and keep no-list-price output unchanged. Tests cover all four price cases.

Changes

Audit AddItem price rendering

Layer / File(s)	Summary
Render sold and list prices web/src/routes/AuditPage.tsx	SalesOrder.AddItem rows now display the sold unit price. Rows with a different list price display both prices. Matching prices retain the “at list” text.
Validate price-case output web/src/routes/AuditPage.test.tsx	Tests assert exact details text for below-list, above-list, at-list, and null-list-price cases. The shared event and text helpers support these assertions.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 8c4ea

Audit details now display sold and list prices as intended for all supported price cases. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main fix: showing the actual sale price instead of the list price. It also uses a conventional commit format.
Description check	✅ Passed	The description provides complete context, rendering requirements, verification commands, mutation evidence, scope, risk, and the linked issue. It does not use the template headings exactly, but it in…
Linked Issues check	✅ Passed	The implementation satisfies issue #758. It renders the actual sale price for below-list and above-list lines, preserves at-list and no-list-price behavior, and adds tests that detect regressions.
Out of Scope Changes check	✅ Passed	The changes are limited to the Audit Details renderer and its tests. They match issue #758 and introduce no unrelated API, payload, migration, translation, or backend changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/758-audit-sale-price

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mforce]

Before / after — captured from a stack rebuilt at each revision

Same viewport (1280×1741, deviceScaleFactor: 1, full page), same signed-in persona, same URL (/audit?action=SalesOrder.AddItem), and the same seeded database — only the app image was rebuilt between the two captures, so the frames differ only where the code does.

The four contract cases are the last five rows of each image.

Row (fixture)	payload sold / list	Before	After
Sim Unpriced Eggs ×24	25 / null	at $0.25	at $0.25 (unchanged)
Sim Small Eggs ×24	40 / 30 (above list)	at list $0.30 ❌	at $0.40 (list $0.30)
Sim Large Eggs ×24	35 / 45 (below list)	at list $0.45 ❌	at $0.35 (list $0.45)
Sim Medium Eggs ×24	38 / 38 (at list)	at list $0.38	at list $0.38 (unchanged)

The two ❌ rows are the defect on a real screen: a line that sold at $0.35 was published as $0.45, and a line that sold above list at $0.40 was published as $0.30 — the audit trail understating it by 25% while claiming it sold at list. The 15 genuine at-list rows above them are byte-identical between the frames, which is the point of holding the fixture constant.

---

This is a rendering defect only — the backend was already correct

Checked directly against the sim database rather than inferred. Across the fixture's 20 SalesOrder.AddItem audit payloads:

unitPriceMinorUnits	listUnitPriceMinorUnits	rows
45	45	15
38	38	2
40	30	1
35	45	1
25	null	1

The writer stores the sale price and the list price as separate, correct fields, and the fixture already exercises a markup, a discount and a null list price. AuditDetails had every number it needed and discarded the sale price at render time. No API, payload, handler or migration change is warranted, and none is in this PR.

_{Capture harness: a throwaway Playwright spec against the tools/simulation stack, deliberately not committed — it writes to /tmp, not docs/images/, so it cannot dirty the tracked screenshots that npm run screenshots owns.}

[mforce]

@coderabbitai review

Context for the review: this is a one-branch rendering fix in web/src/routes/AuditPage.tsx. The SalesOrder.AddItem audit Details cell printed the list price as though it were the price the line sold for, discarding unitPriceMinorUnits whenever a list price existed.

Two things worth aiming at specifically:

1. The replaced test. The previous AddItem test asserted /\$0\.45/ against a payload whose sale price was $0.40 — it pinned the defect. It is replaced by four literal-text assertions, each watched red under a mutation (M1–M3, quoted in the PR body). Please check the new tests actually constrain the behaviour rather than restating the implementation.
2. The no-marker-word decision. An above-list line renders at $0.40 (list $0.30) with no word naming the direction, per EPIC: Discount visibility and control on sales #719's open question 4 (a markup is not a discount). That is deliberate, not an omission.

Backend was verified correct against the seeded database — the payload carries sale and list prices as separate, correct fields — so no API or handler change is in scope here.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== changed files ==='
git diff --name-status HEAD^ HEAD

echo '=== AuditPage diff ==='
git diff --unified=80 HEAD^ HEAD -- web/src/routes/AuditPage.tsx

echo '=== AuditPage test diff ==='
git diff --unified=100 HEAD^ HEAD -- web/src/routes/AuditPage.test.tsx

echo '=== AuditPage outline ==='
ast-grep outline web/src/routes/AuditPage.tsx --items all

echo '=== AuditPage test outline ==='
ast-grep outline web/src/routes/AuditPage.test.tsx --items all

echo '=== price translation keys and renderer references ==='
rg -n -C 4 'detailsAtPrice|detailsAtList|detailsListParen|unitPriceMinorUnits|listUnitPriceMinorUnits' web/src

Length of output: 82857

---

@​mforce No findings.

The SalesOrder.AddItem branch now implements the required contract:

• A null list price renders the sale price only.
• Equal sale and list prices render at list.
• Different sale and list prices render the sale price first, then the parenthesized list price.
• The off-list branch adds no direction marker. This matches the stated markup decision.

The replacement tests constrain the behavior. Each test uses different sale and list values. Each test asserts the full Details cell text. The assertions therefore detect a swapped value, a missing value, an incorrect price order, an incorrect at-list collapse, and the null-list zero-price regression. The above-list test also explicitly rejects direction-marker words.

The existing translation keys support this composition in all three locale files. No API or backend change is indicated.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[mforce]

@coderabbitai full review

Triggering the deep form deliberately, and this is the single deeper pass this PR will take — whatever it returns, the loop stops here.

Why this one earns it. The previous clean pass was CodeRabbit's incremental profile, and the diff touches the money class: it decides which of two prices an audit record publishes. An audit row is evidentiary and not re-derivable from anywhere else in the system, so a defect here is expensive to detect later — which is exactly the case the earlier round already proves, since the original bug shipped through review and a full green suite and only surfaced when #721 printed a discount reason under four rows all claiming to have sold at list.

Worth aiming at, in rough order of what would hurt most:

1. The equality collapse. unit === list is an exact integer compare on minor units. Is there any payload shape where the two are equal in value but not by ===, or where collapsing to at list hides something a reader needs?
2. String composition. The off-list case concatenates two translated strings with a literal space: ${t("detailsAtPrice")} ${t("detailsListParen")}. Check that this holds in es and tl, not just en — per i18n: catalogParity compares key SETS, so a translation that drifts in MEANING passes every gate #688 a locale must never disagree with itself about the same control.
3. The null path. list === null must never render a zero price (Sales: snapshot the list price on the order line (discount foundation) #720). Mutation M3 reproduced exactly that failure (at $0.15 (list $0.00)), so the guard matters.
4. Whether the four tests can pass against a wrong implementation other than the three mutations already run.

Previously verified and not in scope: the backend writes sale and list prices as separate correct fields (checked against the seeded DB — 20 AddItem payloads covering a markup, a discount and a null list), and the before/after screenshots were captured from stacks rebuilt at each revision over one unchanged fixture.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== PR diff: renderer and tests ==='
git diff --unified=100 HEAD^ HEAD -- web/src/routes/AuditPage.tsx web/src/routes/AuditPage.test.tsx

echo '=== Renderer outline ==='
ast-grep outline web/src/routes/AuditPage.tsx --items all

echo '=== Test outline ==='
ast-grep outline web/src/routes/AuditPage.test.tsx --items all

echo '=== Price fields: declarations and renderer/test use ==='
rg -n -C 5 --glob '*.{ts,tsx}' \
  'unitPriceMinorUnits|listUnitPriceMinorUnits|SalesOrder\.AddItem' \
  web/src

echo '=== Relevant translation keys in locale resources ==='
rg -n -C 4 \
  'detailsAtPrice|detailsAtList|detailsListParen' \
  web/src

echo '=== Tests that render AuditPage under non-English locales ==='
rg -n -C 5 --glob '*.{ts,tsx}' \
  'AuditPage|AuditDetails|locale|language|i18n' \
  web/src/routes/AuditPage.test.tsx web/src

Length of output: 201114

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== changed hunks only ==='
git diff --unified=25 HEAD^ HEAD -- web/src/routes/AuditPage.tsx web/src/routes/AuditPage.test.tsx

echo '=== renderer: relevant lines ==='
rg -n -C 18 --glob 'AuditPage.tsx' \
  'SalesOrder\.AddItem|detailsAtPrice|detailsAtList|detailsListParen|unitPriceMinorUnits|listUnitPriceMinorUnits' \
  web/src/routes

echo '=== tests: relevant lines ==='
rg -n -C 22 --glob 'AuditPage.test.tsx' \
  'addItemEvent|detailsText|SalesOrder\.AddItem|at list|list \$|discount|markup|unitPriceMinorUnits|listUnitPriceMinorUnits' \
  web/src/routes

echo '=== API declarations: price field types ==='
rg -n -C 12 --glob 'cluckwork.ts' \
  'unitPriceMinorUnits|listUnitPriceMinorUnits|Audit' \
  web/src/api

echo '=== translation values ==='
for file in web/src/i18n/en.ts web/src/i18n/es.ts web/src/i18n/tl.ts; do
  echo "--- $file ---"
  rg -n -C 2 'detailsAtPrice|detailsAtList|detailsListParen' "$file"
done

echo '=== installed language resources ==='
rg -n -C 4 'RESOURCES|TRANSLATED_NAMESPACES|audit' \
  web/src/i18n/index.ts web/src/i18n/translations-status.ts 2>/dev/null || true

Length of output: 49093

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== AuditPage localization tests ==='
rg -n -C 12 --glob 'AuditPage.test.tsx' \
  'changeLanguage|i18n\.|detailsAtPrice|detailsAtList|detailsListParen|es|tl' \
  web/src/routes

echo '=== Audit payload writer, schema, and contract references ==='
rg -n -C 10 \
  'SalesOrder\.AddItem|listUnitPriceMinorUnits|unitPriceMinorUnits' \
  --glob '*.{cs,json,ts,tsx}' \
  . \
  -g '!web/src/routes/AuditPage.tsx' \
  -g '!web/src/routes/AuditPage.test.tsx' \
  | head -n 1200

echo '=== Price value validation at order-write boundaries ==='
rg -n -C 8 \
  'unitPriceMinorUnits|expectedListUnitPriceMinorUnits|listUnitPriceMinorUnits' \
  --glob '*.cs' \
  . \
  | head -n 1000

Length of output: 201089

---

@​mforce No findings.

The equality check is correct for the accepted payload shape. The renderer accepts only numeric JSON values. It compares minor-unit numbers. There is no supported shape where equal accepted values are unequal with ===.

The null guard is correct. A null or non-numeric listUnitPriceMinorUnits takes the sale-price-only branch. It cannot call money(list).

The off-list composition is correct in all installed languages:

• en: at {{amount}} (list {{amount}})
• es: a {{amount}} (lista {{amount}})
• tl: sa {{amount}} (listahan {{amount}})

The literal space produces the required separation in each case.

The four tests constrain the price contract. They reject a swapped field, a missing sale price, a missing list price, reversed price order, an incorrect at-list collapse, and null rendered as $0.00. The above-list test also rejects a direction marker.

The tests use English text. A future implementation that hardcodes the exact English strings could pass them. The reviewed implementation uses the existing translation keys, and the installed translations support the composition correctly.

⚠️ Action not completed

Review rate limited.

---

Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 27 minutes.