Conversation
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
e9ea7d3 to
27e3edd
Compare
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
27e3edd to
25aedf0
Compare
Confidence Score: 5/5 - Safe to MergeSafe to merge — this release PR for version 0.2.0 appears clean with no issues identified across the reviewed files. The automated review found no logic bugs, security concerns, or correctness problems in any of the 6 reviewed changed files. This looks like a straightforward version bump or release packaging PR with no substantive technical concerns raised. Key Findings:
|
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
25aedf0 to
03c6c45
Compare
|
🧪 Testing To try out this version of the SDK: Expires at: Sat, 23 May 2026 04:09:24 GMT |
🐤 Canary SummaryThis PR enhances CLI error messaging for base URL configuration:
Affected User Flows
|
Confidence Score: 5/5 - Safe to MergeSafe to merge — this PR cleanly delivers the 0.2.0 release with well-scoped additions including Key Findings:
Files requiring special attention
|
🐤 Canary Proposed TestsNo testable user journeys found for this PR. |
Confidence Score: 4/5 - Mostly SafeSafe to merge — this PR introduces well-scoped features including Key Findings:
Files requiring special attention
|
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
| @@ -27,6 +29,8 @@ func TestInnerFlagSet(t *testing.T) { | |||
|
|
|||
| for _, tt := range tests { | |||
| t.Run(tt.name, func(t *testing.T) { | |||
There was a problem hiding this comment.
Correctness: Adding t.Parallel() inside the range loop without capturing tt (via tt := tt) causes all parallel subtests to share the same loop variable — in Go < 1.22, by the time the subtests run, tt will hold the last iteration's value, making all tests use identical inputs.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In file `internal/requestflag/innerflag_test.go`, inside the `for _, tt := range tests` loop in `TestInnerFlagSet`, `t.Parallel()` is called without first capturing the loop variable. For Go versions before 1.22, this causes a data race where all parallel subtests may use the last value of `tt`. Add `tt := tt` immediately before `t.Parallel()` on the line after `t.Run(tt.name, func(t *testing.T) {` to create a per-iteration local copy.
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, boolFlag.Set("bool-flag", "true")) | ||
| assert.Equal(t, true, boolFlag.Get()) | ||
| assert.True(t, boolFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set slice flag with multiple values", func(t *testing.T) { |
There was a problem hiding this comment.
Correctness: Adding t.Parallel() to these subtests while they all share the same strFlag, superstitiousIntFlag, and boolFlag instances (defined in the outer TestFlagSet scope) introduces data races — concurrent calls to PreParse, Set, and Get on the same flag objects will race under -race, causing flaky or incorrect test results.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `internal/requestflag/requestflag_test.go`, the diff adds `t.Parallel()` to subtests inside `TestFlagSet` (around lines 347–393). However, those subtests all operate on the same shared `strFlag`, `superstitiousIntFlag`, and `boolFlag` variables declared in the outer test function. Running them in parallel causes concurrent reads and writes to those shared `Flag` structs, which is a data race.
Fix: Either (a) remove `t.Parallel()` from the subtests that share these outer-scope flag variables, or (b) move the flag construction inside each subtest so each parallel subtest has its own independent instance. The subtests at the bottom of `TestFlagSet` that already create local `sliceFlag` variables are safe to keep parallel.
Confidence Score: 3/5 - Review RecommendedNot safe to merge without fixes — while this PR delivers meaningful improvements like Key Findings:
Files requiring special attention
|
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
03c6c45 to
5683b46
Compare
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
5683b46 to
373f189
Compare
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, boolFlag.Set("bool-flag", "true")) | ||
| assert.Equal(t, true, boolFlag.Get()) | ||
| assert.True(t, boolFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set slice flag with multiple values", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| sliceFlag := &Flag[[]int64]{ | ||
| Name: "slice-flag", | ||
| Default: []int64{}, |
There was a problem hiding this comment.
Correctness: The subtests now run in parallel but all mutate shared strFlag, superstitiousIntFlag, and boolFlag declared in the outer TestFlagSet scope — this introduces data races on Flag internal fields (value, hasBeenSet, applied, count) with no synchronization, causing non-deterministic test failures and potential panics.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In file `internal/requestflag/requestflag_test.go`, the `t.Parallel()` calls added to subtests inside `TestFlagSet` (starting around line 348) cause data races because `strFlag`, `superstitiousIntFlag`, and `boolFlag` are shared mutable state across parallel subtests. Fix this by either: (1) removing `t.Parallel()` from all subtests that share these outer-scope flag variables, or (2) moving the flag declarations inside each subtest so each parallel subtest operates on its own independent flag instance.
| Usage: "The file to ingest.", | ||
| Required: true, | ||
| BodyPath: "file", | ||
| }, | ||
| &requestflag.Flag[any]{ |
There was a problem hiding this comment.
Correctness: Removing FileInput: true means the requestflag package will no longer treat this flag as a file path to read and stream — it will pass the raw string value (the filename) as the body instead of the file contents, silently breaking the upload endpoint.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In pkg/cmd/memory.go around line 375, the `FileInput: true` field was removed from the `requestflag.Flag[string]` struct for the 'file' flag in the `memoriesUpload` command. This field is required for the requestflag package to read the actual file contents from the provided path and pass them to the multipart form upload. Without it, only the raw filename string is sent as the body, breaking file uploads silently. Restore `FileInput: true` to the flag definition.
| @@ -114,6 +116,8 @@ func TestEncode(t *testing.T) { | |||
|
|
|||
| for name, test := range tests { | |||
| t.Run(name, func(t *testing.T) { | |||
There was a problem hiding this comment.
Correctness: Adding t.Parallel() inside the range loop without capturing name and test per iteration causes all subtests to close over the same loop variables — in Go < 1.22 they will all run with the last iteration's values, producing incorrect/flaky test results.
Affected Locations:
- internal/apiquery/query_test.go:118-118
- internal/requestflag/innerflag_test.go:31-31
- internal/requestflag/requestflag_test.go:60-61
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In internal/apiquery/query_test.go, around line 116-117, the for-range loop over `tests` was given a `t.Parallel()` call inside the `t.Run` callback. Without capturing the loop variables locally, all parallel subtests will share the same `name` and `test` variables (the last iteration's values in Go < 1.22), causing incorrect test behaviour. Add `name, test := name, test` immediately after the `t.Run` open brace (before `t.Parallel()`) to shadow and capture each iteration's values.
Confidence Score: 1/5 - Blocking IssuesNot safe to merge — this PR introduces multiple correctness-breaking bugs that will cause non-deterministic test failures and a silent functional regression in production. In Key Findings:
Files requiring special attention
|
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
373f189 to
b0b376a
Compare
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
b0b376a to
4d95a0b
Compare
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, boolFlag.Set("bool-flag", "true")) | ||
| assert.Equal(t, true, boolFlag.Get()) | ||
| assert.True(t, boolFlag.IsSet()) | ||
| }) | ||
|
|
There was a problem hiding this comment.
Correctness: All subtests now run in parallel but share the same strFlag, superstitiousIntFlag, and boolFlag instances defined in the outer scope — concurrent reads and writes to their internal mutable fields (value, hasBeenSet, applied, count) will cause data races detected by go test -race and produce non-deterministic results.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `internal/requestflag/requestflag_test.go`, the subtests inside `TestFlagSet` (starting around line 347) were made parallel via `t.Parallel()`, but they all share the same `strFlag`, `superstitiousIntFlag`, and `boolFlag` variables declared in the outer function scope. These `Flag` structs have mutable fields (`value`, `hasBeenSet`, `applied`, `count`) that are written concurrently, causing data races. Fix this by either: (1) removing `t.Parallel()` from subtests that share these outer variables, or (2) moving the flag construction inside each subtest so each parallel subtest has its own local instance.
| Suggest: true, | ||
| Flags: []cli.Flag{ | ||
| &requestflag.Flag[string]{ | ||
| Name: "file", | ||
| Usage: "The file to ingest.", | ||
| Required: true, | ||
| BodyPath: "file", | ||
| FileInput: true, | ||
| Name: "file", | ||
| Usage: "The file to ingest.", | ||
| Required: true, | ||
| BodyPath: "file", | ||
| }, | ||
| &requestflag.Flag[any]{ | ||
| Name: "collection", |
There was a problem hiding this comment.
Correctness: The FileInput: true field has been removed from the file flag, which likely means the CLI will pass the raw string (file path) instead of reading and streaming the file contents for multipart upload — breaking the memories upload command.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In pkg/cmd/memory.go around line 371-380, the `FileInput: true` field was removed from the `requestflag.Flag[string]` definition for the 'file' flag in the `memoriesUpload` command. This field is required so the CLI reads the file from disk and passes its contents (not just the path string) to the multipart form upload. Restore `FileInput: true` to the flag definition to fix the upload functionality.
Confidence Score: 2/5 - Changes NeededNot safe to merge — this PR introduces three high-severity bugs that must be addressed before merging. In Key Findings:
Files requiring special attention
|
| @@ -114,6 +116,8 @@ func TestEncode(t *testing.T) { | |||
|
|
|||
| for name, test := range tests { | |||
| t.Run(name, func(t *testing.T) { | |||
There was a problem hiding this comment.
Correctness: Adding t.Parallel() inside the subtest causes the closure to capture the test loop variable by reference; in Go < 1.22 all parallel subtests will race on the same test value, producing incorrect or flaky results. A local copy (test := test) is needed before t.Parallel() to pin the value per iteration.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In internal/apiquery/query_test.go, around line 118, the subtest closure added `t.Parallel()` but the `test` loop variable is captured by reference. In Go versions before 1.22, all parallel subtests will share the same loop variable, causing data races and incorrect test behavior. Fix by adding `test := test` immediately before `t.Parallel()` inside the closure to create a per-iteration copy of the loop variable.
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() |
There was a problem hiding this comment.
Correctness: The subtests PreParse initialization, Set string flag, Set int flag with valid value, Set int flag with invalid value, Set int flag with validator failing, and Set bool flag all share the same strFlag, superstitiousIntFlag, and boolFlag pointers declared in the outer TestFlagSet scope. Adding t.Parallel() to these subtests causes concurrent reads and writes to those shared flag structs (mutating value, hasBeenSet, applied, count), introducing data races that will produce non-deterministic failures or corrupt state under -race.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `internal/requestflag/requestflag_test.go`, the subtests inside `TestFlagSet` that were made parallel (lines ~348-386) all mutate shared flag instances (`strFlag`, `superstitiousIntFlag`, `boolFlag`) declared in the outer function scope. This creates data races. Fix this by either: (1) removing `t.Parallel()` from all subtests that share these outer-scope flag variables, or (2) constructing a fresh flag instance inside each subtest instead of sharing the outer-scope ones. The 'Set slice flag' subtests are fine since they already create local flags.
| Suggest: true, | ||
| Flags: []cli.Flag{ | ||
| &requestflag.Flag[string]{ | ||
| Name: "file", | ||
| Usage: "The file to ingest.", | ||
| Required: true, | ||
| BodyPath: "file", | ||
| FileInput: true, | ||
| Name: "file", | ||
| Usage: "The file to ingest.", | ||
| Required: true, | ||
| BodyPath: "file", | ||
| }, | ||
| &requestflag.Flag[any]{ | ||
| Name: "collection", |
There was a problem hiding this comment.
Correctness: Removing FileInput: true means the file flag will no longer be treated as a file path to open and read — it will be passed as a raw string, causing multipart uploads to send the filename string instead of the actual file contents.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In pkg/cmd/memory.go around line 371-380, the `FileInput: true` field was removed from the `requestflag.Flag[string]` definition for the 'file' flag in the `memoriesUpload` command. This field is responsible for instructing the flag processing logic to open the file at the given path and read its contents for multipart upload. Without it, the flag value is treated as a plain string (the filename), not the actual file data, breaking file uploads. Please restore `FileInput: true` to this flag definition.
Confidence Score: 1/5 - Blocking IssuesNot safe to merge — this PR introduces data races in parallelized test suites and a functional regression in multipart file uploads. In Key Findings:
Files requiring special attention
|
Confidence Score: 2/5 - Changes NeededNot safe to merge — this PR introduces multiple concrete correctness bugs that will cause non-deterministic test failures and logic errors in production code. Specifically, Key Findings:
Files requiring special attention
|
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, boolFlag.Set("bool-flag", "true")) | ||
| assert.Equal(t, true, boolFlag.Get()) | ||
| assert.True(t, boolFlag.IsSet()) | ||
| }) | ||
|
|
There was a problem hiding this comment.
Correctness: All six subtests now run in parallel but share the same strFlag, superstitiousIntFlag, and boolFlag pointers declared in the outer TestFlagSet scope — concurrent reads and writes to their internal value, hasBeenSet, applied, and count fields will cause data races and non-deterministic assertions (e.g. strFlag.Get() in "PreParse initialization" may see the value written by "Set string flag" and vice-versa).
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In internal/requestflag/requestflag_test.go, the subtests added in the diff (lines 347-398 of TestFlagSet) all call t.Parallel() but share the mutable flag variables strFlag, superstitiousIntFlag, and boolFlag declared in the outer test function. This introduces data races. Fix by either (a) removing t.Parallel() from every subtest that uses these shared variables, or (b) moving each flag declaration inside its own subtest so there is no shared mutable state.
| for _, tt := range tests { | ||
| t.Run(tt.name+" text", func(t *testing.T) { | ||
| got, err := embedFiles(tt.input, EmbedText) | ||
| t.Parallel() | ||
|
|
||
| got, err := embedFiles(tt.input, EmbedText, nil) | ||
| if tt.wantErr { | ||
| assert.Error(t, err) | ||
| require.Error(t, err) | ||
| } else { | ||
| require.NoError(t, err) | ||
| assert.Equal(t, tt.want, got) | ||
| require.Equal(t, tt.want, got) | ||
| } | ||
| }) | ||
|
|
||
| t.Run(tt.name+" io.Reader", func(t *testing.T) { | ||
| _, err := embedFiles(tt.input, EmbedIOReader) | ||
| t.Parallel() | ||
|
|
||
| _, err := embedFiles(tt.input, EmbedIOReader, nil) | ||
| if tt.wantErr { | ||
| assert.Error(t, err) | ||
| require.Error(t, err) | ||
| } else { | ||
| require.NoError(t, err) | ||
| } |
There was a problem hiding this comment.
Correctness: Adding t.Parallel() inside subtests that close over tt from the range loop creates a classic Go loop-variable capture bug in Go < 1.22 — all parallel subtests will likely reference the final value of tt rather than their intended iteration value, causing wrong inputs to be tested and silently passing incorrect assertions.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In pkg/cmd/flagoptions_test.go, around the for-range loop at line 221, the diff adds t.Parallel() inside subtests that close over the loop variable `tt`. In Go versions before 1.22 this causes all parallel subtests to share the last iteration's `tt` value. Fix this by adding `tt := tt` immediately after the `for _, tt := range tests {` line to shadow and capture the loop variable for each iteration.
Confidence Score: 1/5 - Blocking IssuesNot safe to merge — this PR introduces multiple data race conditions across the test suite, most critically in Key Findings:
Files requiring special attention
|
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, boolFlag.Set("bool-flag", "true")) | ||
| assert.Equal(t, true, boolFlag.Get()) | ||
| assert.True(t, boolFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set slice flag with multiple values", func(t *testing.T) { |
There was a problem hiding this comment.
Correctness: All the parallel subtests share the same strFlag, superstitiousIntFlag, and boolFlag instances declared in the outer TestFlagSet scope — running them concurrently with t.Parallel() introduces data races where one subtest's mutation (e.g. strFlag.Set) races with another's read (e.g. strFlag.Get), causing non-deterministic failures and race detector violations.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In internal/requestflag/requestflag_test.go, the subtests 'PreParse initialization', 'Set string flag', 'Set int flag with valid value', 'Set int flag with invalid value', 'Set int flag with validator failing', and 'Set bool flag' were made parallel by adding t.Parallel(), but they all share the same strFlag, superstitiousIntFlag, and boolFlag variables declared in the outer TestFlagSet function. This creates data races. Fix this by either: (1) removing t.Parallel() from these subtests since they depend on shared state, or (2) declaring separate flag instances inside each subtest so each has its own isolated state.
| @@ -114,6 +116,8 @@ func TestEncode(t *testing.T) { | |||
|
|
|||
| for name, test := range tests { | |||
| t.Run(name, func(t *testing.T) { | |||
There was a problem hiding this comment.
Correctness: Adding t.Parallel() here causes the subtests to capture the loop variables name and test by reference — on Go < 1.22, all parallel subtests will run with the last iteration's values, making the tests non-deterministic or silently testing the wrong inputs.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In file internal/apiquery/query_test.go, at the for-loop starting around line 116, `t.Parallel()` was added inside `t.Run` without re-declaring the loop variables. On Go < 1.22, all parallel subtests will share the same `name` and `test` variables (captured by reference from the outer loop), causing them all to run with the final iteration's values. Fix by adding `name, test := name, test` immediately after the `t.Run(name, func(t *testing.T) {` line and before `t.Parallel()`, so each closure captures its own copy of the loop variables.
Confidence Score: 2/5 - Changes NeededNot safe to merge — this PR introduces widespread data-race and loop-variable-capture bugs across the test suite that will cause non-deterministic test results or silent correctness failures. Specifically, Key Findings:
Files requiring special attention
|
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
97312ab to
e1ca22b
Compare
EntelligenceAI PR SummaryThis PR delivers the v0.2.0 release of the Hyperspell CLI SDK with new features, refactors, and quality improvements.
Confidence Score: 3/5 - Review RecommendedReview recommended — this PR delivers meaningful new features (raw output flag, stdin support, parameter aliasing, file upload propagation) but carries a significant number of unresolved correctness concerns from previous reviews that have not been addressed. Specifically, Key Findings:
Files requiring special attention
|
|
This PR delivers the v0.2.0 release of the Hyperspell CLI, introducing several new capabilities and a broad refactor of the output and flag systems.
|
… many positionals
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
e1ca22b to
d3872d3
Compare
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
d3872d3 to
6a55d31
Compare
|
This PR delivers the v0.2.0 release of the Hyperspell CLI with new features, refactors, and bug fixes.
|
|
This PR delivers the v0.2.0 release of the Hyperspell CLI with significant feature additions, a broad refactoring of the JSON output layer, and test suite improvements.
|
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
6a55d31 to
5e2274b
Compare
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
5e2274b to
9fcce21
Compare
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
9fcce21 to
02a071b
Compare
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
02a071b to
ace8646
Compare
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
ace8646 to
7f35321
Compare
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
7f35321 to
6731d3f
Compare
stainless-app
Bot
force-pushed
the
release-please--branches--main--changes--next
branch
from
6731d3f to
37fae15
Compare
Automated Release PR
0.2.0 (2026-04-23)
Full Changelog: v0.1.0...v0.2.0
Features
-as value representing stdin to binary-only file parameters in CLIs (40329ac)*_BASE_URL/--base-url(665e431)--raw-output/-roption to print raw (non-JSON) strings (4222549)x-stainless-cli-data-alias(fc1ebc7)Bug Fixes
Chores
ShowJSONIterator(5784e71)--format rawbe used in conjunction with--transform(d9c0c77)ShowJSONOptsas argument toformatJSONinstead of many positionals (9e0c109)t.Parallel()(5fc2402)os.Stdoutisn't necessary (b7fd553)os.Chdirtot.Chdir(2f49e45)Documentation
This pull request is managed by Stainless's GitHub App.
The semver version number is based on included commit messages. Alternatively, you can manually set the version number in the title of this pull request.
For a better experience, it is recommended to use either rebase-merge or squash-merge when merging this pull request.
🔗 Stainless website
📚 Read the docs
🙋 Reach out for help or questions