Skip to content

Commit 1418247

Browse files
authored
[Repo Assist] refactor: add strutil.TruncateRunes and remove lookupEnrichmentToken alias (#3574)
🤖 *This PR was created by Repo Assist, an automated AI assistant.* Three code quality improvements identified in #3564 (Semantic Function Clustering Analysis). ## Changes ### 1. Add `strutil.TruncateRunes` (`internal/strutil/truncate.go`) The codebase had a private `truncateForLog` in `internal/proxy/graphql.go` that did rune-aware (Unicode-safe) truncation — essential for non-ASCII content like CJK characters and emoji. This logic is now exposed as `strutil.TruncateRunes(s string, maxRunes int) string`, complementing the existing byte-based `strutil.Truncate`. 9 table-driven tests added covering ASCII, multibyte (CJK), emoji, and edge cases (zero/negative limit, empty string). ### 2. Delegate `truncateForLog` to `strutil.TruncateRunes` (`internal/proxy/graphql.go`) `truncateForLog` now calls `strutil.TruncateRunes`, removing the duplicate implementation and centralising all truncation utilities in the `strutil` package. ### 3. Inline `lookupEnrichmentToken` alias (`internal/server/unified.go`) `lookupEnrichmentToken()` was a zero-value wrapper — it called `envutil.LookupGitHubToken()` and returned the result unchanged. Inlining the single call site removes one unnecessary indirection layer and makes the env-var lookup immediately visible at the call site. (`lookupGitHubAPIBaseURL` is kept as-is since it adds a meaningful default value.) ## Test Status ⚠️ **Infrastructure note**: The Go 1.25.0 toolchain required by `go.mod` is not available in the CI agent environment (blocked by network firewall; only Go 1.24.13 is installed locally). All three changes are syntactically straightforward — no new language features, no interface changes. Tests will run in the GitHub Actions CI environment where Go 1.25.0 is available. ## Trade-offs - `TruncateRunes` uses `[]rune(s)` conversion, which allocates. This is intentional: rune-safe truncation by definition requires decoding all codepoints up to the limit. The function is only used in debug-log paths where this is acceptable. - `truncateForLog` is kept as a package-private shim (rather than inlined) to preserve the semantic name at the two call sites inside `graphql.go`. Closes: no issue (addresses items from analysis report #3564) Ref: #3564 > Generated by [Repo Assist](https://github.com/github/gh-aw-mcpg/actions/runs/24282446110/agentic_workflow) · ● 6.1M · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+repo-assist%22&type=pullrequests) > > To install this [agentic workflow](https://github.com/githubnext/agentics/blob/851905c06e905bf362a9f6cc54f912e3df747d55/workflows/repo-assist.md), run > ``` > gh aw add githubnext/agentics@851905c > ``` <!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, model: auto, id: 24282446110, workflow_id: repo-assist, run: https://github.com/github/gh-aw-mcpg/actions/runs/24282446110 --> <!-- gh-aw-workflow-id: repo-assist -->
2 parents 01145ad + 5c9c6c2 commit 1418247

File tree

4 files changed

+89
-15
lines changed

4 files changed

+89
-15
lines changed

internal/proxy/graphql.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strings"
77

88
"github.com/github/gh-aw-mcpg/internal/logger"
9+
"github.com/github/gh-aw-mcpg/internal/strutil"
910
)
1011

1112
var logGraphQL = logger.New("proxy:graphql")
@@ -191,14 +192,7 @@ var searchQueryArgPattern = regexp.MustCompile(`(?i)\bsearch\s*\(\s*query\s*:\s*
191192

192193
// truncateForLog truncates s to at most maxRunes runes, for safe debug logging.
193194
func truncateForLog(s string, maxRunes int) string {
194-
if maxRunes <= 0 {
195-
return ""
196-
}
197-
r := []rune(s)
198-
if len(r) <= maxRunes {
199-
return s
200-
}
201-
return string(r[:maxRunes])
195+
return strutil.TruncateRunes(s, maxRunes)
202196
}
203197

204198
// extractSearchQuery returns the search query argument from a GraphQL search

internal/server/unified.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ func (g *guardBackendCaller) callCollaboratorPermission(ctx context.Context, arg
279279
return nil, fmt.Errorf("get_collaborator_permission: missing owner/repo/username")
280280
}
281281

282-
token := lookupEnrichmentToken()
282+
token := envutil.LookupGitHubToken()
283283
if token == "" {
284284
logUnified.Printf("get_collaborator_permission: no GitHub token available for %s/%s user %s, skipping", owner, repo, username)
285285
return nil, fmt.Errorf("get_collaborator_permission: no GitHub token available")
@@ -338,12 +338,6 @@ func (g *guardBackendCaller) callCollaboratorPermission(ctx context.Context, arg
338338
return mcpResp, nil
339339
}
340340

341-
// lookupEnrichmentToken searches environment variables for a GitHub token
342-
// suitable for enrichment API calls.
343-
func lookupEnrichmentToken() string {
344-
return envutil.LookupGitHubToken()
345-
}
346-
347341
// lookupGitHubAPIBaseURL returns the GitHub API base URL from environment
348342
// or defaults to https://api.github.com.
349343
func lookupGitHubAPIBaseURL() string {

internal/strutil/truncate.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,18 @@ func TruncateWithSuffix(s string, maxLen int, suffix string) string {
2929
}
3030
return s[:maxLen] + suffix
3131
}
32+
33+
// TruncateRunes truncates s to at most maxRunes Unicode code points (runes).
34+
// Unlike Truncate, which counts bytes, TruncateRunes is safe for non-ASCII
35+
// content (e.g. emoji, CJK characters). If maxRunes is 0 or negative, returns
36+
// an empty string.
37+
func TruncateRunes(s string, maxRunes int) string {
38+
if maxRunes <= 0 {
39+
return ""
40+
}
41+
r := []rune(s)
42+
if len(r) <= maxRunes {
43+
return s
44+
}
45+
return string(r[:maxRunes])
46+
}

internal/strutil/truncate_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,74 @@ func TestTruncateWithSuffix(t *testing.T) {
146146
})
147147
}
148148
}
149+
150+
func TestTruncateRunes(t *testing.T) {
151+
tests := []struct {
152+
name string
153+
input string
154+
maxRunes int
155+
expected string
156+
}{
157+
{
158+
name: "ASCII string within limit",
159+
input: "hello",
160+
maxRunes: 10,
161+
expected: "hello",
162+
},
163+
{
164+
name: "ASCII string exactly at limit",
165+
input: "hello",
166+
maxRunes: 5,
167+
expected: "hello",
168+
},
169+
{
170+
name: "ASCII string exceeds limit",
171+
input: "hello world",
172+
maxRunes: 5,
173+
expected: "hello",
174+
},
175+
{
176+
name: "multibyte runes within limit",
177+
input: "日本語",
178+
maxRunes: 5,
179+
expected: "日本語",
180+
},
181+
{
182+
name: "multibyte runes truncated",
183+
input: "日本語テスト",
184+
maxRunes: 3,
185+
expected: "日本語",
186+
},
187+
{
188+
name: "emoji truncated",
189+
input: "😀😁😂😃😄",
190+
maxRunes: 3,
191+
expected: "😀😁😂",
192+
},
193+
{
194+
name: "zero maxRunes returns empty",
195+
input: "hello",
196+
maxRunes: 0,
197+
expected: "",
198+
},
199+
{
200+
name: "negative maxRunes returns empty",
201+
input: "hello",
202+
maxRunes: -1,
203+
expected: "",
204+
},
205+
{
206+
name: "empty string",
207+
input: "",
208+
maxRunes: 10,
209+
expected: "",
210+
},
211+
}
212+
213+
for _, tt := range tests {
214+
t.Run(tt.name, func(t *testing.T) {
215+
result := TruncateRunes(tt.input, tt.maxRunes)
216+
assert.Equal(t, tt.expected, result)
217+
})
218+
}
219+
}

0 commit comments

Comments
 (0)