Skip to content

refactor: extract helpers to bring 3 functions under the 60-line largefunc limit #42624

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 5 commits into from
Jul 1, 2026
Merged

refactor: extract helpers to bring 3 functions under the 60-line largefunc limit #42624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f9a984b
Initial plan
Copilot Jul 1, 2026
189a0bb
chore: plan largefunc refactors for console, github, parser packages
Copilot Jul 1, 2026
e93a8c1
refactor: extract helpers to reduce largefunc findings in console, gi…
Copilot Jul 1, 2026
6a9b96a
refactor: add doc comments to extracted helper functions
Copilot Jul 1, 2026
372ba91
fix: correct computeValueFirst doc comment and revert unrelated workf…
Copilot Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 26 additions & 23 deletions pkg/console/console.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,29 +215,7 @@ func RenderTable(config TableConfig) string {
}

dataRowCount := len(config.Rows)

styleFunc := func(row, col int) lipgloss.Style {
if !ttyCheck() {
return lipgloss.NewStyle()
}
if row == table.HeaderRow {
headerStyle := styles.TableHeader
return headerStyle.PaddingLeft(1).PaddingRight(1)
}
if config.ShowTotal && len(config.TotalRow) > 0 && row == dataRowCount {
totalStyle := styles.TableTotal
return totalStyle.PaddingLeft(1).PaddingRight(1)
}
if row%2 == 0 {
cellStyle := styles.TableCell
return cellStyle.PaddingLeft(1).PaddingRight(1)
}
return lipgloss.NewStyle().
Foreground(styles.ColorForeground).
Background(styles.ColorTableAltRow).
PaddingLeft(1).
PaddingRight(1)
}
styleFunc := buildTableStyleFunc(config, ttyCheck, dataRowCount)

borderStyle := lipgloss.NewStyle()
if ttyCheck() {
Expand All @@ -257,6 +235,31 @@ func RenderTable(config TableConfig) string {
return output.String()
}

// buildTableStyleFunc returns the lipgloss style function used by RenderTable.
// config supplies the ShowTotal/TotalRow flags; ttyCheck detects terminal output;
// dataRowCount is the number of data rows (excluding any total row).
func buildTableStyleFunc(config TableConfig, ttyCheck func() bool, dataRowCount int) func(int, int) lipgloss.Style {
return func(row, col int) lipgloss.Style {
if !ttyCheck() {
return lipgloss.NewStyle()
}
if row == table.HeaderRow {
return styles.TableHeader.PaddingLeft(1).PaddingRight(1)
}
if config.ShowTotal && len(config.TotalRow) > 0 && row == dataRowCount {
return styles.TableTotal.PaddingLeft(1).PaddingRight(1)
}
if row%2 == 0 {
return styles.TableCell.PaddingLeft(1).PaddingRight(1)
}
return lipgloss.NewStyle().
Foreground(styles.ColorForeground).
Background(styles.ColorTableAltRow).
PaddingLeft(1).
PaddingRight(1)
}
}

// FormatCommandMessage formats a command execution message
func FormatCommandMessage(command string) string {
return applyStyle(styles.Command, "$ ") + command
Expand Down
73 changes: 44 additions & 29 deletions pkg/github/label_objective_mapping.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,43 +64,58 @@ func (om *ObjectiveMapping) ComputeObjectiveValue(issueLabels []string) int {

switch logic {
case "sum":
total := 0
for _, v := range matchingValues {
total += v
}
labelObjectiveMappingLog.Printf("Computed objective value via sum: labels=%v, value=%d", matchedLabels, total)
return total

return om.computeValueSum(matchingValues, matchedLabels)
case "first":
// Return first issue label that's in priority_labels
if len(om.PriorityLabels) > 0 {
for _, issueLabel := range issueLabels {
for _, priorityLabel := range om.PriorityLabels {
if strings.EqualFold(issueLabel, priorityLabel) {
normalizedIssue := strings.ToLower(strings.TrimSpace(issueLabel))
if val, ok := om.LabelToValue[normalizedIssue]; ok {
labelObjectiveMappingLog.Printf("Computed objective value via issue label priority: label=%s, value=%d", issueLabel, val)
return val
}
return om.computeValueFirst(issueLabels, matchingValues, matchedLabels)
default: // "max"
return om.computeValueMax(matchingValues, matchedLabels)
}
}

// computeValueSum adds all matching label values and logs the result.
func (om *ObjectiveMapping) computeValueSum(matchingValues []int, matchedLabels []string) int {
total := 0
for _, v := range matchingValues {
total += v
}
labelObjectiveMappingLog.Printf("Computed objective value via sum: labels=%v, value=%d", matchedLabels, total)
return total
}

// computeValueFirst returns the value for the first issue label that appears in PriorityLabels.
// It iterates issueLabels in their existing order and returns the value for the first one
// found in PriorityLabels; if none match, it falls back to the first value in matchingValues.
func (om *ObjectiveMapping) computeValueFirst(issueLabels []string, matchingValues []int, matchedLabels []string) int {
// Return first issue label that's in priority_labels
if len(om.PriorityLabels) > 0 {
for _, issueLabel := range issueLabels {
for _, priorityLabel := range om.PriorityLabels {
if strings.EqualFold(issueLabel, priorityLabel) {
normalizedIssue := strings.ToLower(strings.TrimSpace(issueLabel))
if val, ok := om.LabelToValue[normalizedIssue]; ok {
labelObjectiveMappingLog.Printf("Computed objective value via issue label priority: label=%s, value=%d", issueLabel, val)
return val
}
}
}
}
// Fallback to first matching label
result := matchingValues[0]
labelObjectiveMappingLog.Printf("Computed objective value via first match: labels=%v, value=%d", matchedLabels, result)
return result
}
// Fallback to first matching label
result := matchingValues[0]

@github-actions github-actions Bot Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] computeValueFirst (and computeValueMax at line 111) silently require len(matchingValues) > 0 but neither the signature nor the doc comment states this precondition.

💡 Suggestion

The caller (ComputeObjectiveValue) guards against this at line 56, so it is safe today. However, since these are now standalone methods, a future caller could pass an empty slice and get a panic with no obvious explanation. Add the precondition to the doc comment:

// computeValueFirst returns the value for the highest-priority matching label.
// It walks issueLabels in priority order; if none match, it falls back to the
// first label in matchingValues.
// Precondition: len(matchingValues) > 0.
func (om *ObjectiveMapping) computeValueFirst(...) int {

Same addition applies to computeValueMax.

@copilot please address this.

labelObjectiveMappingLog.Printf("Computed objective value via first match: labels=%v, value=%d", matchedLabels, result)
return result
}

default: // "max"
maxVal := matchingValues[0]
for _, v := range matchingValues {
if v > maxVal {
maxVal = v
}
// computeValueMax returns the highest value among all matching labels.
func (om *ObjectiveMapping) computeValueMax(matchingValues []int, matchedLabels []string) int {
maxVal := matchingValues[0]
for _, v := range matchingValues {
if v > maxVal {
maxVal = v
}
labelObjectiveMappingLog.Printf("Computed objective value via max: labels=%v, value=%d", matchedLabels, maxVal)
return maxVal
}
labelObjectiveMappingLog.Printf("Computed objective value via max: labels=%v, value=%d", matchedLabels, maxVal)
return maxVal
}

// DefaultObjectiveMapping returns the built-in default label-to-value mapping.
Expand Down
47 changes: 25 additions & 22 deletions pkg/parser/schema_suggestions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,10 +650,22 @@ func findFieldLocationsInSchema(schemaDoc any, targetField, currentPath string)
allLocations := collectSchemaPropertyPaths(schemaDoc, "", 0)
targetLower := strings.ToLower(targetField)

seen := make(map[string]struct {
})
exactMatches := collectExactFieldMatches(allLocations, targetField, currentPath)
if len(exactMatches) > 0 {
schemaSuggestionsLog.Printf("Found %d exact schema locations for field '%s'", len(exactMatches), targetField)
return exactMatches
}

// Collect exact matches first
fuzzyMatches := collectFuzzyFieldMatches(allLocations, targetLower, currentPath)
schemaSuggestionsLog.Printf("Found %d fuzzy schema locations for field '%s'", len(fuzzyMatches), targetField)
return fuzzyMatches
}

// collectExactFieldMatches returns all locations in allLocations where the field
// name case-insensitively equals targetField, excluding the currentPath location.
// Duplicate (fieldName, schemaPath) pairs are suppressed.
func collectExactFieldMatches(allLocations []schemaFieldLocation, targetField, currentPath string) []schemaFieldLocation {
seen := make(map[string]struct{})
var exactMatches []schemaFieldLocation
for _, loc := range allLocations {
if loc.SchemaPath == currentPath {
Expand All @@ -663,43 +675,36 @@ func findFieldLocationsInSchema(schemaDoc any, targetField, currentPath string)
if setutil.Contains(seen, key) {
continue
}
seen[key] = struct {
}{}

seen[key] = struct{}{}
if strings.EqualFold(loc.FieldName, targetField) {
loc.Distance = 0
exactMatches = append(exactMatches, loc)
}
}
return exactMatches
}

if len(exactMatches) > 0 {
schemaSuggestionsLog.Printf("Found %d exact schema locations for field '%s'", len(exactMatches), targetField)
return exactMatches
}

// Fall back to fuzzy matching with a stricter distance threshold for high confidence
seenFuzzy := make(map[string]struct {
})
// collectFuzzyFieldMatches returns locations whose field names are within
// maxPathSearchDistance Levenshtein edits of targetLower, excluding currentPath.
// Results are sorted by distance ascending, then by schema path for stable output.
func collectFuzzyFieldMatches(allLocations []schemaFieldLocation, targetLower, currentPath string) []schemaFieldLocation {
seen := make(map[string]struct{})
var fuzzyMatches []schemaFieldLocation
for _, loc := range allLocations {
if loc.SchemaPath == currentPath {
continue
}
key := loc.FieldName + "|" + loc.SchemaPath
if setutil.Contains(seenFuzzy, key) {
if setutil.Contains(seen, key) {
continue
}
seenFuzzy[key] = struct {
}{}

seen[key] = struct{}{}
dist := LevenshteinDistance(targetLower, strings.ToLower(loc.FieldName))
if dist > 0 && dist <= maxPathSearchDistance {
loc.Distance = dist
fuzzyMatches = append(fuzzyMatches, loc)
}
}

// Sort fuzzy matches by distance (ascending), then path for stable output
slices.SortFunc(fuzzyMatches, func(a, b schemaFieldLocation) int {
if a.Distance != b.Distance {
if a.Distance < b.Distance {
Expand All @@ -716,8 +721,6 @@ func findFieldLocationsInSchema(schemaDoc any, targetField, currentPath string)
return 0
}
})

schemaSuggestionsLog.Printf("Found %d fuzzy schema locations for field '%s'", len(fuzzyMatches), targetField)
return fuzzyMatches
}

Expand Down
Loading