From d8b4d3ab878cce453edb62e62b85c3c016264ac5 Mon Sep 17 00:00:00 2001 From: IamAngus <110540546+IamAngusU@users.noreply.github.com> Date: Mon, 5 Oct 2026 09:17:50 +0200 Subject: [PATCH] Fix mixed decimal notation in local calculator requests --- CHANGELOG.md | 7 +++ README.md | 9 ++++ cmd/contextbridge/local_tools_test.go | 12 ++++- internal/localtools/localtools.go | 49 +++++++++++++++--- internal/localtools/localtools_test.go | 71 ++++++++++++++++++++++++++ 5 files changed, 141 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc7db8e..3a730bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +- Fixed local arithmetic rejecting mixed decimal commas and points in separate + numbers, such as `(4,2 * 10.1) * 3 / 30 + 5`. The shared `do`/chat/MCP resolver + now normalizes each literal independently and accepts unambiguous comma + decimals in English/bare expressions. German comma conventions remain intact; + ambiguous thousands-style commas and malformed/grouped literals fail without + model fallback. Exact rational arithmetic and resource limits are unchanged. + - Accept strict structured adapter instructions in fresh agent proposals before policy validation and hash approval. Existing saved-plan representation, profile restrictions and adapter validation remain unchanged; non-adapter diff --git a/README.md b/README.md index bdcf6e0..bbced8d 100644 --- a/README.md +++ b/README.md @@ -174,6 +174,15 @@ or unsupported requests are not extracted into partial calculations; this is not yet a general autonomous tool-selection loop. MCP clients can explicitly use the same bounded resolver through `contextbridge.local_tool` with a `prompt` string. +Decimal commas and points may be mixed **between numbers**: `cb do "Was macht +(4,2 * 10.1) mal 3 / 30 + 5?"` computes exactly `9.242`. Results use decimal +points (or an exact fraction). Unambiguous comma decimals such as `4,2` also work +in English and bare expressions. German requests retain their comma-decimal +convention; without that context, `1,234` is ambiguous and asks for an ungrouped +integer (`1234`) or a decimal point (`1.234`). Thousands grouping such as +`1.234,56` or `1,234.56` is not supported: write `1234.56`. Malformed or ambiguous +recognized arithmetic remains a calculator error, never an LLM fallback. + Run `cb do` without a prompt for an interactive session. `cluster chat` remains the explicit surface for provider, model, profile, artifact, E2EE, and budget overrides; `do` accepts those same flags when they are needed. Put flags before diff --git a/cmd/contextbridge/local_tools_test.go b/cmd/contextbridge/local_tools_test.go index 2d5c441..4573442 100644 --- a/cmd/contextbridge/local_tools_test.go +++ b/cmd/contextbridge/local_tools_test.go @@ -14,6 +14,12 @@ func TestChatLocalToolsDoNotSubmitToPool(t *testing.T) { if err := state.turn(context.Background(), "Was macht 10 mal 3 / 30?"); err != nil { t.Fatal(err) } + if err := state.turn(context.Background(), "Was macht (4,2 * 10.1) mal 3 / 30 + 5?"); err != nil { + t.Fatal(err) + } + if err := state.turn(context.Background(), "What is 1,234 + 0.5?"); err == nil { + t.Fatal("ambiguous thousands separator accepted") + } if err := state.turn(context.Background(), "Calculate 1/0"); err == nil { t.Fatal("division by zero accepted") } @@ -49,11 +55,15 @@ func TestMCPLocalToolWorksWithoutServiceAndStrictlyRejectsEffects(t *testing.T) `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"test","version":"1"}}}`, `{"jsonrpc":"2.0","method":"notifications/initialized"}`, `{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"contextbridge.local_tool","arguments":{"prompt":"Was macht 10 mal 3 / 30?"}}}`, + `{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"contextbridge.local_tool","arguments":{"prompt":"Was macht (4,2 * 10.1) mal 3 / 30 + 5?"}}}`, ) if result := toolStructured(t, responses[1]); result["tool"] != "calculator" || result["text"] != "10 * 3 / 30 = 1" { t.Fatal(result) } - for _, raw := range []string{`{"prompt":"1+1","shell":"echo x"}`, `{"prompt":"1+1","Prompt":"2+2"}`, `{"prompt":"write file.txt"}`, `{"prompt":null}`, `{"prompt":"1 / 0"}`} { + if result := toolStructured(t, responses[2]); result["tool"] != "calculator" || result["text"] != "(4,2 * 10.1) * 3 / 30 + 5 = 9.242" { + t.Fatalf("mixed-decimal MCP result: %#v", result) + } + for _, raw := range []string{`{"prompt":"1+1","shell":"echo x"}`, `{"prompt":"1+1","Prompt":"2+2"}`, `{"prompt":"write file.txt"}`, `{"prompt":null}`, `{"prompt":"1 / 0"}`, `{"prompt":"What is 1,234 + 0.5?"}`} { if _, err := callLocalTool(json.RawMessage(raw)); err == nil { t.Errorf("accepted %s", raw) } diff --git a/internal/localtools/localtools.go b/internal/localtools/localtools.go index 1bfd12c..03a8355 100644 --- a/internal/localtools/localtools.go +++ b/internal/localtools/localtools.go @@ -86,12 +86,9 @@ func Resolve(prompt string) (result Result, handled bool, err error) { return Result{}, false, nil } result = Result{Tool: "calculator", Input: strings.TrimSpace(text)} - if strings.Contains(text, ",") { - // Commas without a German request are ambiguous (decimal vs thousands). - if !german || strings.Contains(text, ".") { - return result, true, errors.New("calculator: ambiguous decimal separators; use decimal points without thousands separators") - } - text = strings.ReplaceAll(text, ",", ".") + text, err = normalizeDecimalLiterals(text, german) + if err != nil { + return result, true, fmt.Errorf("calculator: %w", err) } value, err := Evaluate(text) if err != nil { @@ -101,6 +98,46 @@ func Resolve(prompt string) (result Result, handled bool, err error) { return result, true, nil } +// Normalize each numeric literal independently: 4,2 * 10.1 is not a grouped +// number. Dots retain their existing decimal meaning. A German request keeps +// its existing comma-decimal preference; otherwise a comma with a plausible +// thousands group (1,234) requires clarification rather than a numeric guess. +// Grouping separators are never silently removed, including within German +// requests. Evaluate remains the bounded, dot-only rational arithmetic parser. +func normalizeDecimalLiterals(text string, german bool) (string, error) { + if !strings.Contains(text, ",") { + return text, nil + } + var normalized strings.Builder + for pos := 0; pos < len(text); { + start := pos + for pos < len(text) && (text[pos] >= '0' && text[pos] <= '9' || text[pos] == '.' || text[pos] == ',') { + pos++ + } + if pos == start { + normalized.WriteByte(text[pos]) + pos++ + continue + } + literal := text[start:pos] + if strings.Contains(literal, ",") { + if strings.Count(literal, ",") != 1 || strings.Contains(literal, ".") { + return "", errors.New("ambiguous decimal separators within one number; use ungrouped numbers such as 1234.56") + } + integer, fraction, _ := strings.Cut(literal, ",") + if fraction == "" { + return "", errors.New("invalid decimal number; a decimal comma needs fractional digits") + } + if !german && len(integer) >= 1 && len(integer) <= 3 && integer[0] != '0' && len(fraction) == 3 { + return "", errors.New("ambiguous comma (decimal or thousands); write 1234 for an integer or 1.234 for a decimal") + } + literal = integer + "." + fraction + } + normalized.WriteString(literal) + } + return normalized.String(), nil +} + // Evaluate is a bounded rational-arithmetic parser, never eval or a subprocess. // Supported operators: + - * / ^ and parentheses; powers are integer -64..64. func Evaluate(expression string) (*big.Rat, error) { diff --git a/internal/localtools/localtools_test.go b/internal/localtools/localtools_test.go index e5bc815..c43ac98 100644 --- a/internal/localtools/localtools_test.go +++ b/internal/localtools/localtools_test.go @@ -33,6 +33,51 @@ func TestDoNotHijackOtherIntent(t *testing.T) { } } +func TestDecimalSeparatorsAreInterpretedPerNumber(t *testing.T) { + for prompt, want := range map[string]string{ + "Was macht (4,2 * 10.1) mal 3 / 30 + 5?": "9.242", + "Was macht (4.2 * 10,1) mal 3 / 30 + 5?": "9.242", + "Was ergibt 0,1 + 0.2?": "0.3", + "Was ergibt -4,2 * (10.1 + 0,9)?": "-46.2", + "What is (4,2 * 10.1) times 3 / 30 + 5?": "9.242", + "Calculate 0,125 + 0.875": "1", + "Calculate 1234,567 + 0.433": "1235", + "4,2 * 10.1": "42.42", + "4,2 mal 10.1": "42.42", + ",5 + .5": "1", + "What is -,5 * 2.0?": "-1", + "Was ist 1,234 + 0.766?": "2", + "Was ist 1,000 + 0.25?": "1.25", + "1.234 + 0.766": "2", + "Was ist 1 / 0,3?": "10/3", + "What is 4,2 + 0?": "4.2", + } { + t.Run(prompt, func(t *testing.T) { + result, handled, err := Resolve(prompt) + if !handled || err != nil || result.Tool != "calculator" || !strings.HasSuffix(result.Text, " = "+want) { + t.Fatalf("got %#v handled=%v error=%v; want %s", result, handled, err, want) + } + }) + } +} + +func TestAmbiguousOrMalformedDecimalLiteralsNeverFallBack(t *testing.T) { + for _, prompt := range []string{ + "Calculate 1,000 + 1", "1,234 + 0.5", "What is -12,345 + 0.1?", + "What is 123,456 / 2.0?", "Calculate , + 2", "Was ist 1, + 2?", + "Was ist 1,2,3 + 2?", "Calculate 1,000,000 + 0.5", + "Was ist 1.234,56 + 2?", "What is 1,234.56 + 2?", + "Was ist 1,2.3 + 4?", "Calculate 1..2 + 0,1", "Was ist 1, 2 + 3?", + "Was ist 1 / 0,0?", "Was ist 0,0 ^ 0?", + } { + t.Run(prompt, func(t *testing.T) { + if result, handled, err := Resolve(prompt); !handled || err == nil || result.Tool != "calculator" { + t.Fatalf("wanted recognized calculator error, got %#v handled=%v error=%v", result, handled, err) + } + }) + } +} + func TestRecognizedErrorsNeverFallBackToModel(t *testing.T) { for _, prompt := range []string{"Calculate 1 / 0", "Calculate 0 ^ 0", "Calculate 2 ^ 1000000", "Calculate (3 + 2", "Calculate 2 3", "Calculate 1,000 + 1", "Calculate 1.2.3 + 2", "Calculate " + strings.Repeat("(", 40) + "1" + strings.Repeat(")", 40), "Calculate " + strings.Repeat("9", 101), "Calculate (2^64)^64"} { if _, handled, err := Resolve(prompt); !handled || err == nil { @@ -70,3 +115,29 @@ func FuzzEvaluate(f *testing.F) { } }) } + +func FuzzDecimalNormalization(f *testing.F) { + for _, seed := range []string{"(4,2 * 10.1) * 3 / 30 + 5", "1,234", "0,125", ",5", "1,2.3", "1,", "1/0", "1,000,000"} { + f.Add(seed, false) + f.Add(seed, true) + } + f.Fuzz(func(t *testing.T, text string, german bool) { + if len(text) > 4096 || !numericExpression.MatchString(text) { + return + } + normalized, err := normalizeDecimalLiterals(text, german) + if err != nil { + return + } + if strings.Contains(normalized, ",") || len(normalized) != len(text) { + t.Fatalf("normalization lost expression structure: %q -> %q", text, normalized) + } + again, err := normalizeDecimalLiterals(normalized, german) + if err != nil || again != normalized { + t.Fatal("normalization is not idempotent") + } + if value, err := Evaluate(normalized); err == nil && value == nil { + t.Fatal("missing computed result") + } + }) +}