Skip to content

chore: cleanup xxx ptr to use lo#1238

Merged
looplj merged 1 commit intorelease/v0.9.xfrom
dev-tmp
Apr 1, 2026
Merged

chore: cleanup xxx ptr to use lo#1238
looplj merged 1 commit intorelease/v0.9.xfrom
dev-tmp

Conversation

@looplj
Copy link
Copy Markdown
Owner

@looplj looplj commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request standardizes the creation of pointers in test files by replacing various custom helper functions with the lo.ToPtr utility from the samber/lo library. Several critical compilation errors were identified in channel_llm_codex_oauth_test.go and performance_test.go where the built-in new function was incorrectly used with values (e.g., new("hi"), new(true)) instead of types. These instances must be corrected to use lo.ToPtr to ensure the code compiles and aligns with the rest of the changes.

Model: "gpt-4o-mini",
Messages: []llm.Message{
{Role: "user", Content: llm.MessageContent{Content: loPtr("hi")}},
{Role: "user", Content: llm.MessageContent{Content: new("hi")}},
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.

critical

The new built-in function in Go takes a type as an argument (e.g., new(string)), not a value. Passing a string literal like "hi" will cause a compilation error. Since the goal of this PR is to use the lo library, you should use lo.ToPtr("hi") instead. Note that you will also need to add "github.com/samber/lo" to the imports in this file.

Suggested change
{Role: "user", Content: llm.MessageContent{Content: new("hi")}},
{Role: "user", Content: llm.MessageContent{Content: lo.ToPtr("hi")}},

{
name: "streaming request - Stream is true",
streamValue: boolPtr(true),
streamValue: new(true),
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.

critical

The new built-in function takes a type, not a value. new(true) is invalid Go syntax and will fail to compile. Use lo.ToPtr(true) instead. Note that you will also need to add "github.com/samber/lo" to the imports in this file.

Suggested change
streamValue: new(true),
streamValue: lo.ToPtr(true),

{
name: "non-streaming request - Stream is false",
streamValue: boolPtr(false),
streamValue: new(false),
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.

critical

Similar to the previous case, new(false) is invalid syntax. Use lo.ToPtr(false) instead.

Suggested change
streamValue: new(false),
streamValue: lo.ToPtr(false),

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

View 3 additional findings in Devin Review.

Open in Devin Review

Model: "gpt-4o-mini",
Messages: []llm.Message{
{Role: "user", Content: llm.MessageContent{Content: loPtr("hi")}},
{Role: "user", Content: llm.MessageContent{Content: new("hi")}},
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.

🔴 new("hi") used instead of lo.ToPtr("hi") — Go builtin new takes a type, not a value

The Go builtin new function takes a type as its argument (e.g., new(string) returns *string pointing to ""). Passing a value like "hi" is invalid Go syntax and won't compile. This also violates the backend rule in .agent/rules/backend.md:72: "Use lo.ToPtr to get a pointer to a constant value instead of implementing helper functions like stringPtr." The correct replacement for the removed loPtr("hi") should be lo.ToPtr("hi").

Prompt for agents
In internal/server/biz/channel_llm_codex_oauth_test.go, line 67, replace `new("hi")` with `lo.ToPtr("hi")`. Also add `"github.com/samber/lo"` to the import block at the top of the file (around line 3-22).
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

{
name: "streaming request - Stream is true",
streamValue: boolPtr(true),
streamValue: new(true),
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.

🔴 new(true) used instead of lo.ToPtr(true) — Go builtin new takes a type, not a value

The Go builtin new function takes a type as its argument (e.g., new(bool) returns *bool pointing to false). Passing the value true is invalid Go syntax and won't compile. This also violates the backend rule in .agent/rules/backend.md:72: "Use lo.ToPtr to get a pointer to a constant value instead of implementing helper functions like stringPtr." The correct replacement for the removed boolPtr(true) should be lo.ToPtr(true).

Prompt for agents
In internal/server/orchestrator/performance_test.go, replace `new(true)` on line 33 with `lo.ToPtr(true)`, and `new(false)` on line 38 with `lo.ToPtr(false)`. Also add `"github.com/samber/lo"` to the import block at the top of the file (around lines 3-14).
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

{
name: "non-streaming request - Stream is false",
streamValue: boolPtr(false),
streamValue: new(false),
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.

🔴 new(false) used instead of lo.ToPtr(false) — Go builtin new takes a type, not a value

Same issue as new(true) on line 33. The Go builtin new takes a type, not a value — new(false) is invalid Go. This violates the backend rule in .agent/rules/backend.md:72 to use lo.ToPtr. The correct replacement for the removed boolPtr(false) should be lo.ToPtr(false).

Suggested change
streamValue: new(false),
streamValue: lo.ToPtr(false),
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@looplj looplj merged commit 78f244d into release/v0.9.x Apr 1, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant