Skip to content

Commit 4aea625

Browse files
committed
fix(calc): reject weight-unit and NaN finenesses instead of silently haircutting (om-t0fs)
Adversarial review, two CONFIRMED findings — both the same silent-garbage class this bead already fixed for the 1.0 = full-melt case. Finding 2 — a DECIMAL point laundered a weight into a junk haircut. The weight rule only guarded BARE numbers, but "0.4 oz" / ".9 oz" have a decimal point, so finenessFraction treated them as marked and discarded the trailing unit as annotation: "0.4 oz" -> 0.40 -> 40% bucket -> factor 0.80 (main returned 1.0), silently, no anomaly. "40 grain" was protected; "0.4 oz" was not — inconsistent by construction. Now a weight-unit token (oz, ozt, g, gram(s), gr, grain(s)) ANYWHERE in the tail makes the whole string unparseable, marked or not, so it takes the conservative branch AND raises the fineness anomaly. Non-unit annotation is untouched: ".900 coin", "90% (worn)", "80% (CAD)", "22k .9167" still parse. Finding 4 — NaN escaped the range check. ParseFloat("nan") is a valid NaN, not an error, and `f <= 0 || f > 1` is false for NaN (every comparison is), so "nan%" classified as a parsed fraction -> factor 1.0, no anomaly. Inverted the accept test to `!(f > 0 && f <= 1)`, which is true for NaN and +Inf, routing both to conservative + anomaly. Tests: +TestWeightUnitFinenessIsAnomaly (weight strings + nan% -> conservative factor AND one fineness anomaly each), +TestMarkedFinenessAnnotationStillParses (the over-rejection boundary: marked finenesses with non-unit annotation still parse, no anomaly), plus cases in the finenessFraction and buybackFactor tables. No existing test weakened. AC7 unchanged: /api/summary over sample-data is byte-identical to the pre-review branch, and vs main still differs only by the additive "anomalies": [].
1 parent 94d1987 commit 4aea625

2 files changed

Lines changed: 109 additions & 3 deletions

File tree

internal/calc/calc.go

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,14 +224,27 @@ const (
224224
// stands alone or is followed by an explicit fineness word. A number carrying any
225225
// other unit is a WEIGHT, and mistaking it for a fineness is what haircut this lot
226226
// at the 40% rate. A number that *is* explicitly marked (%, ".", karat) may be
227-
// followed by free annotation — "80% (CAD)" is a real fineness with a note on it.
227+
// followed by free annotation — "80% (CAD)" is a real fineness with a note on it —
228+
// EXCEPT a weight unit: a decimal point must not launder "0.4 oz" into 40% by
229+
// discarding the "oz" as annotation (om-t0fs review, finding 2). A weight unit
230+
// anywhere in the tail means the whole string is a weight, marked or not.
228231
func finenessFraction(raw string) (float64, fineClass) {
229232
fields := strings.Fields(strings.ToLower(raw))
230233
if len(fields) == 0 {
231234
return 0, fineUnstated
232235
}
233236
head, rest := fields[0], fields[1:]
234237

238+
// A weight unit token anywhere after the number ("0.4 oz", ".9 oz bar",
239+
// "40 grain") makes this a WEIGHT, not a fineness — regardless of whether a
240+
// decimal point marked the number. Non-unit annotation (".900 coin", "90%
241+
// (worn)") is not a weight and stays parseable.
242+
for _, tok := range rest {
243+
if isWeightUnit(tok) {
244+
return 0, fineUnparseable
245+
}
246+
}
247+
235248
percent := strings.HasSuffix(head, "%")
236249
head = strings.TrimSuffix(head, "%")
237250
karat := false
@@ -283,12 +296,30 @@ func finenessFraction(raw string) (float64, fineClass) {
283296
f = n / 10000
284297
}
285298
}
286-
if f <= 0 || f > 1 {
287-
return 0, fineUnparseable // not a fine fraction: "900%", "40k", "-1"
299+
// Invert the accept test rather than reject with `f <= 0 || f > 1`: NaN fails
300+
// every comparison, so the reject form let "nan%" (ParseFloat("nan") is a valid
301+
// NaN, not an error) through as a parsed fraction -> full melt, the silent
302+
// flattering answer again (om-t0fs review, finding 4). `f > 0 && f <= 1` is
303+
// false for NaN and +Inf, so both route to the conservative branch + an anomaly.
304+
if !(f > 0 && f <= 1) {
305+
return 0, fineUnparseable // not a fine fraction: "900%", "40k", "-1", "nan%"
288306
}
289307
return f, fineParsed
290308
}
291309

310+
// isWeightUnit reports whether tok is a mass/weight unit word. A "fineness" whose
311+
// annotation is actually a weight unit ("0.4 oz", "40 grain", ".4 g") is a weight,
312+
// not a fineness, and must not be bucketed as junk silver (om-t0fs). tok is already
313+
// lower-cased by finenessFraction.
314+
func isWeightUnit(tok string) bool {
315+
switch tok {
316+
case "oz", "ozt", "g", "gram", "grams", "gr", "grain", "grains":
317+
return true
318+
default:
319+
return false
320+
}
321+
}
322+
292323
// The junk-silver buckets, as RANGES over the normalized fine fraction. The
293324
// tolerance is wide enough for notation noise (".900" == "90%" == "900") and tight
294325
// enough to keep the things that are NOT US junk silver out of the buckets:

internal/calc/calc_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,16 @@ func TestBuybackFactorClassification(t *testing.T) {
455455
{"40 grain is a weight", "silver", "40 grain", worst},
456456
{"90 grain is a weight", "silver", "90 grain", worst},
457457
{"pure gibberish", "silver", "junk", worst},
458+
// Review finding 2 — a DECIMAL-marked weight is still a weight, not junk.
459+
{"0.4 oz is a weight", "silver", "0.4 oz", worst},
460+
{"point-9 oz is a weight", "silver", ".9 oz", worst},
461+
{"point-4 g is a weight", "silver", ".4 g", worst},
462+
{"0.4 oz round is a weight", "silver", "0.4 oz round", worst},
463+
// Review finding 4 — NaN must not classify as a parseable fraction (1.0).
464+
{"nan% is not a fineness", "silver", "nan%", worst},
465+
// Review boundary — a marked fineness with NON-unit annotation still parses.
466+
{"point-900 with annotation", "silver", ".900 coin", f90},
467+
{"90% with parenthetical note", "silver", "90% (worn)", f90},
458468
// AC3 — the metal guard normalizes: a 'Silver' lot reaches the haircut.
459469
{"capitalized Silver 90%", "Silver", "90%", f90},
460470
{"shouty SILVER 90%", "SILVER", ".900", f90},
@@ -533,13 +543,27 @@ func TestFinenessFraction(t *testing.T) {
533543
{"22k .9167", 22.0 / 24.0, fineParsed},
534544
{"80% (CAD)", 0.80, fineParsed},
535545
{"90% (pre-1965)", 0.90, fineParsed},
546+
{".900 coin", 0.900, fineParsed}, // annotation after a marked number
547+
{"90% (worn)", 0.90, fineParsed},
536548
{"", 0, fineUnstated},
537549
{" ", 0, fineUnstated},
538550
{"40 grain", 0, fineUnparseable},
539551
{"junk", 0, fineUnparseable},
540552
{"-", 0, fineUnparseable},
541553
{"900%", 0, fineUnparseable}, // 9.0 fine is not a thing
542554
{"0", 0, fineUnparseable},
555+
// om-t0fs review finding 2 — a decimal-MARKED weight is still a weight: the
556+
// unit word must not be discarded as annotation and the number bucketed.
557+
{"0.4 oz", 0, fineUnparseable},
558+
{".9 oz", 0, fineUnparseable},
559+
{".4 g", 0, fineUnparseable},
560+
{"0.4 oz round", 0, fineUnparseable}, // unit word even with more text after
561+
{".9 oz bar", 0, fineUnparseable},
562+
{"5 grams", 0, fineUnparseable},
563+
{"90 gr", 0, fineUnparseable},
564+
// om-t0fs review finding 4 — NaN/Inf must not slip past the range check.
565+
{"nan%", 0, fineUnparseable},
566+
{"inf", 0, fineUnparseable},
543567
} {
544568
t.Run("fineness="+tc.in, func(t *testing.T) {
545569
got, class := finenessFraction(tc.in)
@@ -641,3 +665,54 @@ func TestSampleReportNoAnomalies(t *testing.T) {
641665
t.Errorf("clean sample fixture produced anomalies: %+v", a)
642666
}
643667
}
668+
669+
// TestWeightUnitFinenessIsAnomaly closes om-t0fs review findings 2 and 4. A
670+
// decimal point used to flip a WEIGHT string into "marked number, discard the
671+
// unit" — so "0.4 oz" silently took a 40% junk haircut (main returned 1.0), no
672+
// anomaly. And a NaN slipped past the range check as a parsed fraction -> 1.0. Both
673+
// are the silent-flattering-garbage failure mode this whole bead is about: each
674+
// must now take the CONSERVATIVE factor AND raise a fineness anomaly, exactly like
675+
// "40 grain".
676+
func TestWeightUnitFinenessIsAnomaly(t *testing.T) {
677+
s := model.DefaultSettings()
678+
worst := math.Min(1.0, math.Min(s.SilverBuyback40pct, s.SilverBuyback90pct)) // 0.80
679+
for _, fineness := range []string{"0.4 oz", ".9 oz", ".4 g", "0.4 oz round", ".9 oz bar", "5 grams", "90 gr", "nan%"} {
680+
t.Run("fineness="+fineness, func(t *testing.T) {
681+
l := model.Lot{ID: 7, Activity: "crh", Product: "mislabelled find", Metal: "silver", Fineness: fineness, Qty: 1, FineOzEach: 0.1}
682+
if got := buybackFactor(l, s); math.Abs(got-worst) > tol {
683+
t.Errorf("buybackFactor(fineness=%q) = %.4f, want conservative %.4f (a weight/NaN is not junk silver)", fineness, got, worst)
684+
}
685+
as := Compute(model.Dataset{Spot: model.Spot{SilverUSD: 60}, Settings: s, Lots: []model.Lot{l}}).Anomalies
686+
if len(as) != 1 || as[0].Field != "fineness" || as[0].Value != fineness {
687+
t.Errorf("fineness=%q anomalies = %+v, want exactly one fineness anomaly carrying that value", fineness, as)
688+
}
689+
})
690+
}
691+
}
692+
693+
// TestMarkedFinenessAnnotationStillParses is the over-rejection boundary the
694+
// reviewer flagged for finding 2: the unit-word rejection must catch weight units
695+
// WITHOUT eating a legitimate marked fineness whose trailing token is ordinary
696+
// annotation. These must still parse, bucket normally, and raise NO anomaly.
697+
func TestMarkedFinenessAnnotationStillParses(t *testing.T) {
698+
s := model.DefaultSettings()
699+
for _, tc := range []struct {
700+
fineness string
701+
want float64
702+
}{
703+
{".900 coin", 0.90}, // annotation, not a unit
704+
{"90% (worn)", 0.90}, // parenthetical note
705+
{"80% (CAD)", 1.00}, // world coin, outside the junk buckets
706+
{"22k .9167", 1.00}, // karat; .9167 is outside the 90% band -> full melt
707+
} {
708+
t.Run("fineness="+tc.fineness, func(t *testing.T) {
709+
l := model.Lot{Activity: "crh", Metal: "silver", Fineness: tc.fineness, Qty: 1, FineOzEach: 1}
710+
if got := buybackFactor(l, s); math.Abs(got-tc.want) > tol {
711+
t.Errorf("buybackFactor(fineness=%q) = %.4f, want %.4f", tc.fineness, got, tc.want)
712+
}
713+
if a := Compute(model.Dataset{Spot: model.Spot{SilverUSD: 60}, Settings: s, Lots: []model.Lot{l}}).Anomalies; len(a) != 0 {
714+
t.Errorf("fineness=%q raised anomalies %+v, want none (it parses)", tc.fineness, a)
715+
}
716+
})
717+
}
718+
}

0 commit comments

Comments
 (0)