fix(calc): work a nested set-up out inside-out

pkg/calc/calc.go

@@ -46,6 +46,12 @@ type Operand struct {
 	// Found is false when the number appears on no evidence block: the
 	// figure is the model's, not the document's.
 	Found bool `json:"found"`
+	// Derived is true when the number is the result of a calculation nested
+	// inside this one: it comes from that calculation's operands, which are
+	// traced there, not from the evidence.
+	Derived bool `json:"derived,omitempty"`
+
+	pos int // byte offset of the number in its expression
 }
 
 // Calculation is one evaluated <<expression>>.
@@ -57,32 +63,118 @@ type Calculation struct {
 	Operands   []Operand `json:"operands,omitempty"`
 }
 
-var exprRE = regexp.MustCompile(`(?s)<<(.+?)>>`)
-
 // Resolve replaces every <<expression>> in text with its value followed by
 // the expression, "63.8634 (= 365 * ...)", so a reader sees both. One that
 // does not evaluate stays visible with a note, never a made-up number.
+//
+// A set-up may nest another (<<65,398 / <<(50,710 + 47,528) / 2>>>>): the
+// innermost is worked out first and its value feeds the one around it
+// (HAL-3245). Every level is recorded. An inner one that fails makes its
+// outer one fail. A << with no >>, or a >> with no <<, is not computable,
+// never a partial number.
 func Resolve(text string) (string, []Calculation) {
-	var calcs []Calculation
-	out := exprRE.ReplaceAllStringFunc(text, func(m string) string {
-		expr := strings.TrimSpace(m[2 : len(m)-2])
-		c := Calculation{Expression: expr}
-		ops, err := operands(expr)
-		if err == nil {
-			c.Operands = ops
+	type open struct {
+		start   int   // where the << starts in buf
+		derived []int // where inner results were written into it, in buf
+	}
+	var (
+		calcs []Calculation
+		buf   strings.Builder
+		opens []open
+	)
+	// cut takes an open set-up's text out of buf, trimmed, with the
+	// positions of its inner results relative to that text.
+	cut := func(o open) (string, map[int]bool) {
+		all := buf.String()
+		buf.Reset()
+		buf.WriteString(all[:o.start])
+		raw := all[o.start:]
+		lead := len(raw) - len(strings.TrimLeft(raw, " \t\n"))
+		at := map[int]bool{}
+		for _, d := range o.derived {
+			at[d-o.start-lead] = true
 		}
-		v, err := Evaluate(expr)
-		if err != nil {
-			c.Error = err.Error()
+		return strings.TrimSpace(raw), at
+	}
+	for i := 0; i < len(text); {
+		switch {
+		case strings.HasPrefix(text[i:], "<<"):
+			opens = append(opens, open{start: buf.Len()})
+			i += 2
+		case strings.HasPrefix(text[i:], ">>"):
+			i += 2
+			if len(opens) == 0 {
+				calcs = append(calcs, Calculation{Expression: ">>", Error: "a >> with no <<"})
+				continue
+			}
+			o := opens[len(opens)-1]
+			opens = opens[:len(opens)-1]
+			expr, derived := cut(o)
+			nested := len(opens) > 0
+			c := evaluate(expr, derived)
 			calcs = append(calcs, c)
-			return expr + " (could not be computed)"
+			switch {
+			case c.Value == nil && nested:
+				// the text stays, so the set-up around it fails too
+				buf.WriteString("(" + expr + ")")
+			case c.Value == nil:
+				buf.WriteString(expr + " (could not be computed)")
+			case nested:
+				parent := &opens[len(opens)-1]
+				p := plain(*c.Value)
+				at := buf.Len()
+				if strings.HasPrefix(p, "-") {
+					at++ // the operand's number starts after its sign
+				}
+				parent.derived = append(parent.derived, at)
+				buf.WriteString(p)
+			default:
+				buf.WriteString(c.Display + " (= " + expr + ")")
+			}
+		default:
+			buf.WriteByte(text[i])
+			i++
 		}
-		c.Value = &v
-		c.Display = Format(v)
-		calcs = append(calcs, c)
-		return c.Display + " (= " + expr + ")"
-	})
-	return out, calcs
+	}
+	for len(opens) > 0 { // a << never closed: everything after it is one failed set-up
+		o := opens[len(opens)-1]
+		opens = opens[:len(opens)-1]
+		expr, _ := cut(o)
+		calcs = append(calcs, Calculation{Expression: expr, Error: "a << with no >>"})
+		buf.WriteString(expr + " (could not be computed)")
+	}
+	return buf.String(), calcs
+}
+
+// evaluate is one set-up as a Calculation: its operands, and its value or
+// why it has none. derived holds the byte positions in expr where inner
+// results were written; the operands that start there are marked Derived.
+func evaluate(expr string, derived map[int]bool) Calculation {
+	c := Calculation{Expression: expr}
+	if ops, err := operands(expr); err == nil {
+		for i := range ops {
+			ops[i].Derived = derived[ops[i].pos]
+		}
+		c.Operands = ops
+	}
+	v, err := Evaluate(expr)
+	if err != nil {
+		c.Error = err.Error()
+		return c
+	}
+	c.Value = &v
+	c.Display = Format(v)
+	return c
+}
+
+// plain writes an inner result back into its outer expression at full
+// precision, in a form the number grammar reads (no exponent, no grouping).
+func plain(v float64) string {
+	s := strconv.FormatFloat(v, 'f', -1, 64) // shortest form that reads back as v
+	if s == "-0" {
+		return "0"
+	}
+	return s
 }
 
 // Source is one evidence block an operand may come from.
@@ -101,6 +193,9 @@ func Trace(calcs []Calculation, sources []Source) {
 	for ci := range calcs {
 		for oi := range calcs[ci].Operands {
 			op := &calcs[ci].Operands[oi]
+			if op.Derived {
+				continue // traced in the calculation that produced it
+			}
 			for i := range sources {
 				if containsNumber(norm[i], op) {
 					op.Evidence, op.Page, op.Found = i+1, sources[i].Page, true
@@ -159,6 +254,7 @@ type token struct {
 	op   byte
 	num  float64
 	text string
+	pos  int // byte offset in the expression
 }
 
 // numberRE reads one number: an optional currency sign (a space may follow
@@ -168,7 +264,9 @@ type token struct {
 var numberRE = regexp.MustCompile(`^(?:[$€£]\s?)?(?:(?:\d{1,3}(?:,\d{3})+|\d+)(?:\.\d+)?|\.\d+)%?`)
 
 func tokenize(expr string) ([]token, error) {
-	expr = strings.NewReplacer("×", "*", "÷", "/", "−", "-", "–", "-").Replace(expr)
+	// Same byte length in and out, so a token's offset is its offset in
+	// the expression as written ("×" and "÷" are 2 bytes, "−" and "–" 3).
+	expr = strings.NewReplacer("×", "* ", "÷", "/ ", "−", "-  ", "–", "-  ").Replace(expr)
 	var toks []token
 	for i := 0; i < len(expr); {
 		c := expr[i]
@@ -193,7 +291,7 @@ func tokenize(expr string) ([]token, error) {
 			if err != nil {
 				return nil, err
 			}
-			toks = append(toks, token{kind: tNum, num: v, text: strings.TrimSpace(m)})
+			toks = append(toks, token{kind: tNum, num: v, text: strings.TrimSpace(m), pos: i})
 			i += len(m)
 		}
 	}
@@ -359,7 +457,7 @@ func operands(expr string) ([]Operand, error) {
 				value = -value
 			}
 		}
-		out = append(out, Operand{Text: text, Value: value})
+		out = append(out, Operand{Text: text, Value: value, pos: t.pos})
 	}
 	return out, nil
 }

pkg/calc/calc_test.go

@@ -129,3 +129,94 @@ func TestOperandsKeepTheirSign(t *testing.T) {
 		t.Errorf("-1,234 not found as (1,234): %+v", calcs[0].Operands[0])
 	}
 }
+
+// FinanceBench 04412 (Lockheed Martin), as the model wrote it (HAL-3245).
+func TestNestedSetUpIsWorkedOutInsideOut(t *testing.T) {
+	out, calcs := Resolve("Ratio = <<65,398 / <<(50,710 + 47,528) / 2>>>>")
+	if out != "Ratio = 1.3314 (= 65,398 / 49119)" {
+		t.Fatalf("out = %q", out)
+	}
+	if len(calcs) != 2 || calcs[0].Expression != "(50,710 + 47,528) / 2" || calcs[1].Expression != "65,398 / 49119" {
+		t.Fatalf("calcs = %+v", calcs)
+	}
+	if math.Abs(*calcs[1].Value-65398.0/49119.0) > 1e-9 {
+		t.Errorf("value = %v", *calcs[1].Value)
+	}
+}
+
+func TestTwoLevelsAndSiblings(t *testing.T) {
+	out, calcs := Resolve("x <<<<1 + 1>> * <<2 + 3>> + <<<<4>> / 2>>>> y")
+	if out != "x 12 (= 2 * 5 + 2) y" || len(calcs) != 5 {
+		t.Errorf("out = %q, %d calcs", out, len(calcs))
+	}
+}
+
+func TestFailedInnerSetUpFailsTheOuter(t *testing.T) {
+	out, calcs := Resolve("<<10 / <<abs(1)>>>>")
+	if out != "10 / (abs(1)) (could not be computed)" {
+		t.Errorf("out = %q", out)
+	}
+	for _, c := range calcs {
+		if c.Value != nil {
+			t.Errorf("a level has a value: %+v", c)
+		}
+	}
+}
+
+func TestUnbalancedMarkerIsNeverANumber(t *testing.T) {
+	for raw, want := range map[string]string{
+		"margin <<11,512 / 86,392 * 100":   "margin 11,512 / 86,392 * 100 (could not be computed)",
+		"margin 11,512 / 86,392 * 100>> %": "margin 11,512 / 86,392 * 100 %",
+	} {
+		out, calcs := Resolve(raw)
+		if out != want || len(calcs) == 0 || calcs[len(calcs)-1].Value != nil {
+			t.Errorf("%q -> %q, %+v", raw, out, calcs)
+		}
+	}
+}
+
+func TestInnerResultKeepsItsPrecision(t *testing.T) {
+	if out, _ := Resolve("<<<<1 / 3>> * 3>>"); out != "1 (= 0.3333333333333333 * 3)" {
+		t.Errorf("out = %q", out)
+	}
+}
+
+// An inner result written into its outer set-up is marked derived and not
+// looked for on the pages; a literal of the same value stays a literal.
+func TestInnerResultIsDerivedNotModelSupplied(t *testing.T) {
+	_, calcs := Resolve("<<<<1 + 1>> + 2>>")
+	outer := calcs[1]
+	if outer.Expression != "2 + 2" || len(outer.Operands) != 2 {
+		t.Fatalf("outer = %+v", outer)
+	}
+	if !outer.Operands[0].Derived || outer.Operands[1].Derived {
+		t.Errorf("want the first 2 derived and the literal 2 not: %+v", outer.Operands)
+	}
+	Trace(calcs, []Source{{Page: 3, Text: "two items, 2 in all"}})
+	if outer.Operands[0].Found || !calcs[1].Operands[1].Found {
+		t.Errorf("trace: %+v", calcs[1].Operands)
+	}
+	// a negative inner result, behind a 3-byte minus sign in the outer text
+	_, neg := Resolve("<<10 − <<2 - 5>> × 2>>")
+	ops := neg[1].Operands
+	if len(ops) != 3 || ops[0].Derived || !ops[1].Derived || ops[1].Value != -3 || ops[2].Derived {
+		t.Errorf("operands = %+v", ops)
+	}
+	if *neg[1].Value != 16 {
+		t.Errorf("value = %v", *neg[1].Value)
+	}
+}
+
+func TestInnerResultKeepsFullPrecision(t *testing.T) {
+	for expr, want := range map[string]float64{
+		"<<<<1 / 3>> * 1000000000000>>":              1.0 / 3 * 1e12,
+		"<<1 / <<0.0000000000001 * 1>>>>":            1e13,
+		"<<<<1.234567 / 1000000000>> * 1000000000>>": 1.234567,
+	} {
+		_, calcs := Resolve(expr)
+		last := calcs[len(calcs)-1]
+		if last.Value == nil || math.Abs(*last.Value-want) > 1e-9*math.Max(1, math.Abs(want)) {
+			t.Errorf("%s = %+v, want %v", expr, last, want)
+		}
+	}
+}