From c9af5f8400c8b1cfdf649c23b4fff3785044659d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 18:59:33 +0000 Subject: [PATCH 1/2] Initial plan From eceb791de72f69c51512253f3aae38668bb070b6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 19:14:04 +0000 Subject: [PATCH 2/2] Add warning logs for float-to-int truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add logger instances to map_helpers.go, metrics.go, and safe_outputs.go - Update parseIntValue() to warn when float truncation occurs - Update ConvertToInt() to warn when float truncation occurs - Update max-patch-size handling to warn when float truncation occurs - Add comprehensive unit tests for truncation scenarios - Warning format: "Float value X.XX truncated to integer Y" - No warnings for clean conversions (60.0 → 60) - All existing tests pass Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_test.go | 10 ++++ pkg/workflow/map_helpers.go | 11 ++++- pkg/workflow/map_helpers_test.go | 72 ++++++++++++++++++++++++++++ pkg/workflow/metrics.go | 7 ++- pkg/workflow/metrics_test.go | 82 ++++++++++++++++++++++++++++++++ pkg/workflow/safe_outputs.go | 4 ++ 6 files changed, 184 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/compiler_test.go b/pkg/workflow/compiler_test.go index c2d552fa04f..9c2e1c00280 100644 --- a/pkg/workflow/compiler_test.go +++ b/pkg/workflow/compiler_test.go @@ -5332,6 +5332,16 @@ func TestExtractSafeOutputsMaximumPatchSize(t *testing.T) { }, expected: 2048, }, + { + name: "float64 with fractional part truncates to int", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "max-patch-size": 1536.7, + "create-pull-request": nil, + }, + }, + expected: 1536, + }, { name: "custom value as uint64 (from YAML parsing)", frontmatter: map[string]any{ diff --git a/pkg/workflow/map_helpers.go b/pkg/workflow/map_helpers.go index 4962bf0b93b..6b8605c520e 100644 --- a/pkg/workflow/map_helpers.go +++ b/pkg/workflow/map_helpers.go @@ -1,5 +1,9 @@ package workflow +import "github.com/githubnext/gh-aw/pkg/logger" + +var mapHelpersLog = logger.New("workflow:map_helpers") + // parseIntValue safely parses various numeric types to int // This is a common utility used across multiple parsing functions func parseIntValue(value any) (int, bool) { @@ -11,7 +15,12 @@ func parseIntValue(value any) (int, bool) { case uint64: return int(v), true case float64: - return int(v), true + intVal := int(v) + // Warn if truncation occurs (value has fractional part) + if v != float64(intVal) { + mapHelpersLog.Printf("Float value %.2f truncated to integer %d", v, intVal) + } + return intVal, true default: return 0, false } diff --git a/pkg/workflow/map_helpers_test.go b/pkg/workflow/map_helpers_test.go index 895c2f246af..b666e69f75e 100644 --- a/pkg/workflow/map_helpers_test.go +++ b/pkg/workflow/map_helpers_test.go @@ -68,6 +68,78 @@ func TestParseIntValue(t *testing.T) { } } +// TestParseIntValueTruncation tests float truncation scenarios +func TestParseIntValueTruncation(t *testing.T) { + tests := []struct { + name string + value float64 + expected int + shouldTruncate bool + }{ + { + name: "clean conversion - no truncation", + value: 60.0, + expected: 60, + shouldTruncate: false, + }, + { + name: "truncation required - 60.5", + value: 60.5, + expected: 60, + shouldTruncate: true, + }, + { + name: "truncation required - 60.7", + value: 60.7, + expected: 60, + shouldTruncate: true, + }, + { + name: "clean conversion - 100.0", + value: 100.0, + expected: 100, + shouldTruncate: false, + }, + { + name: "truncation required - 123.99", + value: 123.99, + expected: 123, + shouldTruncate: true, + }, + { + name: "truncation required - negative with fraction", + value: -5.5, + expected: -5, + shouldTruncate: true, + }, + { + name: "clean conversion - negative integer", + value: -10.0, + expected: -10, + shouldTruncate: false, + }, + { + name: "truncation required - small fraction", + value: 1.1, + expected: 1, + shouldTruncate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, ok := parseIntValue(tt.value) + if !ok { + t.Errorf("parseIntValue() should return ok=true for float64") + } + if result != tt.expected { + t.Errorf("parseIntValue(%v) = %v, want %v", tt.value, result, tt.expected) + } + // Note: We can't directly test if warning was logged, but we verify the conversion is correct + }) + } +} + func TestFilterMapKeys(t *testing.T) { tests := []struct { name string diff --git a/pkg/workflow/metrics.go b/pkg/workflow/metrics.go index 7298aaf9bec..5a7c7703d37 100644 --- a/pkg/workflow/metrics.go +++ b/pkg/workflow/metrics.go @@ -249,7 +249,12 @@ func ConvertToInt(val any) int { case int64: return int(v) case float64: - return int(v) + intVal := int(v) + // Warn if truncation occurs (value has fractional part) + if v != float64(intVal) { + metricsLog.Printf("Float value %.2f truncated to integer %d", v, intVal) + } + return intVal case string: if i, err := strconv.Atoi(v); err == nil { return i diff --git a/pkg/workflow/metrics_test.go b/pkg/workflow/metrics_test.go index c31a5d9e353..eb3d240e6b5 100644 --- a/pkg/workflow/metrics_test.go +++ b/pkg/workflow/metrics_test.go @@ -949,3 +949,85 @@ func TestFinalizeToolCallsAndSequence(t *testing.T) { }) } } + +// TestConvertToIntTruncation tests float truncation scenarios in ConvertToInt +func TestConvertToIntTruncation(t *testing.T) { + tests := []struct { + name string + val float64 + expected int + shouldTruncate bool + }{ + { + name: "clean conversion - no truncation", + val: 60.0, + expected: 60, + shouldTruncate: false, + }, + { + name: "truncation required - 60.5", + val: 60.5, + expected: 60, + shouldTruncate: true, + }, + { + name: "truncation required - 60.7", + val: 60.7, + expected: 60, + shouldTruncate: true, + }, + { + name: "clean conversion - 100.0", + val: 100.0, + expected: 100, + shouldTruncate: false, + }, + { + name: "truncation required - 123.99", + val: 123.99, + expected: 123, + shouldTruncate: true, + }, + { + name: "truncation required - negative with fraction", + val: -5.5, + expected: -5, + shouldTruncate: true, + }, + { + name: "clean conversion - negative integer", + val: -10.0, + expected: -10, + shouldTruncate: false, + }, + { + name: "truncation required - small fraction", + val: 1.1, + expected: 1, + shouldTruncate: true, + }, + { + name: "clean conversion - zero", + val: 0.0, + expected: 0, + shouldTruncate: false, + }, + { + name: "truncation required - 0.9", + val: 0.9, + expected: 0, + shouldTruncate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ConvertToInt(tt.val) + if result != tt.expected { + t.Errorf("ConvertToInt(%v) = %v, want %v", tt.val, result, tt.expected) + } + // Note: We can't directly test if warning was logged, but we verify the conversion is correct + }) + } +} + diff --git a/pkg/workflow/safe_outputs.go b/pkg/workflow/safe_outputs.go index 5ddf1666f00..d7cf90eb186 100644 --- a/pkg/workflow/safe_outputs.go +++ b/pkg/workflow/safe_outputs.go @@ -547,6 +547,10 @@ func (c *Compiler) extractSafeOutputsConfig(frontmatter map[string]any) *SafeOut } case float64: intVal := int(v) + // Warn if truncation occurs (value has fractional part) + if v != float64(intVal) { + safeOutputsLog.Printf("max-patch-size: float value %.2f truncated to integer %d", v, intVal) + } if intVal >= 1 { config.MaximumPatchSize = intVal }