fix(sales): keep a line's discount markers agreeing while its price is edited (#752)

[mforce]

Closes #752.

Fixes three of the five findings deferred from PR #741. The other two are product decisions, recorded below and on the issue.

Findings 1 and 2 were one defect

The issue lists them separately. They share a root cause, so they got one fix.

A row's tint (tr.discounted) and its Below list chip read the saved line, while the Discount cell rendered a hardcoded "—" for the whole edit. One row, two sources of truth. Two consequences:

• A below-list line under edit said "discounted" in two places and "not measurable" in a third, simultaneously.
• An above-list line, whose only marker lives in that cell, lost its marker entirely for as long as the edit was open. That is the state most likely to be misread, because the user is mid-change.

The row now resolves one discount, and the tint, the chip and the cell all read it. While that row is being edited it describes the typed price, following the #445 precedent in this same file where the eggs column tracks the edited quantity instead of going blank. An unparseable price box (mid-keystroke, empty) falls back to the saved line rather than flickering to No list price.

The cell markup is now one discountCell value rendered by both the editing and non-editing branches. Two near-identical copies drifting apart is precisely what produced this bug, so collapsing them is part of the fix rather than tidying.

Finding 5

A percent above 0 but below 0.05 rendered 0.0% beside a non-zero amount. New discountPercent() helper renders <0.1% below the rendering threshold, built from fmt.count(0.1, 1) rather than a string literal so the decimal separator stays the locale's, since es writes 0,1. Applied at every percent render site, the live typing hints included, because the flaw was identical at each and fixing one would have left the others.

No new i18n keys. The form substitutes into the existing {{percent}} placeholder, so en/es/tl, the Help page and GLOSSARY.md are unchanged. Nothing user-visible gained new wording.

Deliberately not fixed

• Finding 3, an all-above-list order rendering an em dash. Needs new wording in three locales plus Help and glossary sync. That is a product decision about what such an order should say, not a defect.
• Finding 4, no hover state on a discounted row. The issue itself records it as possibly correct, and a new rule also touches the stylesheet equality-set guards. It deserves a decision, not a reflex.

Both are recorded as decisions on #752 rather than left silent.

Evidence

Three new tests. Reverting SalesPage.tsx alone turns 2 of the 3 RED:

× keeps the tint, the chip and the Discount cell agreeing as the price is edited
× says <0.1% rather than 0.0% when a real discount rounds below the rendered precision
  Tests  2 failed | 1 passed

Stated plainly: the third test passes against the original code too. It guards the new live-discount fallback; it does not catch the original bug. Three new tests is not three bugs caught.

Gates and the #662 before/after follow in a comment once the sim stack is rebuilt at this head.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9b905350-bff6-4b15-af04-977c199b3b9e

---

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]

Gates at ecd986a, run in an isolated worktree

Run from ~/.cluckwork-slices/752/worktree rather than the primary checkout, because another session is working on its own branch in that shared directory and switched it mid-run. Isolating removed the contention.

=== TYPECHECK ===   tsc -b --noEmit          exit 0
=== BUILD ===       precache 66 entries (1264.08 KiB), dist/sw.js generated
=== FULL SUITE ===  Test Files  122 passed (122)
                    Tests  2783 passed (2783)      63.63s

The red proof, verbatim

Reverting web/src/routes/SalesPage.tsx alone, leaving the new tests in place:

 × keeps the tint, the chip and the Discount cell agreeing as the price is edited   344ms
 × says <0.1% rather than 0.0% when a real discount rounds below the rendered precision   102ms
   Tests  2 failed | 1 passed | 187 skipped (190)

The third test, falls back to the saved line rather than flickering when the price box is unparseable, passes on the original code too. It guards the new live-discount fallback rather than catching the original defect. Recorded so nobody reads three new tests as three bugs caught.

#662 evidence is pending, and here is exactly why

This changes what is on screen, so the rule applies. The capture is not done yet: the sim stack on this machine had been up 11 hours, built from #741's head, which is the precise failure #662's "rebuilt at the head under review" clause exists to prevent — it would have served bytes that do not contain this fix while looking entirely plausible.

A rebuild at ecd986a is running now. The before/after will be attached as a comment with gh pr comment --attach, not hosted on a branch.

Worth recording: my first check reported "no sim containers running" and that was wrong. docker ps was failing with a permission error that my shell swallowed, so an empty result read as an empty stack. The shell's group list predated this account's docker group membership; sg docker -c picks it up without a re-login.

[mforce]

#662 — 1:1 before/after, both stacks rebuilt from source

Two full sim-stack rebuilds, no reuse: BEFORE at d26d389 (current main, pre-fix), AFTER at ecd986a (this PR's head). Same order, same row, same typed price, same viewport, captured by the same scripted spec against both builds so the only variable is the code.

Captured at 1:1 (1280px viewport, no downscaling), per #662.

Case 1 — a line typed BELOW its list price, mid-edit

Sim Small Eggs, list $0.30, typed 0.20, quantity 24. That is a real 33.3% discount.

BEFORE. The Discount cell reads —. No row tint, no chip. The screen says nothing at all about a line being sold at a third off.

AFTER. The row tints, the Below list chip appears beside the product, and the Discount cell reads $2.40 · 33.3%. All three markers agree, live against the typed price. ($0.10 per unit × 24 = $2.40; $0.10 ÷ $0.30 = 33.3%.)

Case 2 — an ABOVE-list line, mid-edit

Same row, typed 1.00 against the $0.30 list. This is finding 2, and it is the more dangerous one: an above-list line's only marker lives in the Discount cell.

BEFORE. —. The line is being sold well over list and carries no marker anywhere on screen for the whole edit.

AFTER. The cell reads Above list, the same wording the non-editing row uses, because both branches now render one shared value.

---

One honest caveat. The order reference differs between the pairs (SO-A29F9282 vs SO-09F28E02) because reset.sh reseeds the fixture on each rebuild and references are generated per seed. The product, list price, quantity, typed price and row position are identical, which is what the comparison turns on.

Capture method. Scripted, not hand-driven, so it is re-runnable: a spec under tools/simulation/ui/specs-screenshots/ that signs in as the Sales persona through the real login form, opens the first Draft order, and drives the row into the editor. Personas come from the git-ignored cast file and every selector resolves through the SPA's own catalogs, per the suite's standing no-hardcoded-credential and no-hardcoded-English rules. The spec is not committed — it writes to /tmp for this attachment rather than refreshing a committed image, which is the boundary playwright.screenshots.config.ts draws for itself.

Images uploaded with gh pr comment --attach, so they depend on no branch and survive any cleanup.