[NIT] The field name http on AICoreClient (and Client) shadows the standard library package name net/http. While this compiles fine and is a common pattern in the stdlib itself (e.g. http.Client), it can be slightly surprising. Not blocking.
[NIT] The anthropicRequest struct alignment was changed: AnthropicVersion was added and Model changed from json:"model" to json:"model,omitempty". This is correct for the AI Core path (model is omitted because the deployment already specifies it), but the direct Anthropic path in completeAnthropic relies on Model being present. Since completeAnthropic always sets reqBody.Model = c.model, the omitempty will never trigger for a non-empty model — so it's functionally correct, but slightly confusing that the same struct now has different semantics for the two code paths.
[MINOR] fetchDeployments is called while holding the write lock (c.mu.Lock()), and it performs network I/O. Although the design comment acknowledges the pre-fetch token strategy, fetchDeployments itself still executes an HTTP request under the lock. This means concurrent callers waiting for the lock are blocked during the full deployment discovery round-trip. For CI usage this is acceptable (comment in code says so), but worth noting for any future longer-lived use.
[MINOR] Duplicate doc comment on getDeploymentURL. The function has two consecutive doc comment blocks starting with '// getDeploymentURL returns the deployment URL for a model, fetching deployments if needed.' The first is a stale copy that should be removed. This is visible in both the diff and the full file.
[NIT] The time import is used only in TestAICoreClient_TokenExpiry and TestAICoreClient_WithTimeout. The fmt import is used only in TestAICoreClient_TokenExpiry. These are fine — just noting the test file is in package llm (white-box), which is appropriate for testing unexported methods like getToken and getDeploymentURL.
[NIT] PR_NUMBER and AUTHOR are derived from github.event context expressions interpolated directly into a shell script. While these values come from trusted Gitea event payloads (not user-supplied strings in the PR body/title), it is generally safer to pass context values through environment variables (already done for GITEA_TOKEN) rather than inline shell interpolation, to guard against unexpected characters. The current form is low-risk but worth noting.
[MINOR] SELF_REVIEWED_LABEL_ID=37 is a hardcoded numeric ID that is opaque and fragile. If the label is ever recreated (different ID) or this workflow is reused in another repository, it will silently fail to remove the label (the DELETE returns a non-2xx which is swallowed by '
[MINOR] Duplicate doc comment on getDeploymentURL: the function comment appears twice (lines 143-144 and 145-151). The first one-liner is a leftover from before the fuller comment was added.
[MINOR] CompleteAnthropic has a hardcoded maxTokens parameter in its signature but callers always pass 8192. This leaks an implementation detail through the API; consider making it a constant or removing the parameter since it's always 8192.
[NIT] The field name http shadows the standard library package name net/http. Within the AICoreClient methods, http.MethodPost etc. still work because the package is imported separately, but this naming can confuse readers. The parent Client struct in client.go uses the same pattern, so it's consistent — but worth noting.
[NIT] The token refresh threshold time.Now().Add(5*time.Minute).Before(c.tokenExpiry) is somewhat inverted in readability. c.tokenExpiry.After(time.Now().Add(5*time.Minute)) expresses the same condition more naturally: "expiry is more than 5 minutes in the future".
[NIT] The struct literal alignment for anthropicRequest fields is inconsistent in the diff — AnthropicVersion and Model are right-aligned differently from the remaining fields. A gofmt run would normalize this (though it may already be correct after formatting; hard to tell from the diff context).
[NIT] Duplicate doc comment: // getDeploymentURL returns the deployment URL for a model, fetching deployments if needed. appears twice consecutively (lines ~154-155 and ~156-160 in the source). One copy should be removed.
[NIT] Test uses json.NewEncoder(w).Encode(...) without checking the error return. While test code is less critical, this pattern appears throughout the test file. A helper like mustEncodeJSON(t, w, v) would be cleaner and consistent.
[MINOR] When multiple models map to the same name (e.g. two RUNNING gpt-5 deployments in TestAICoreClient_DeploymentFetch), fetchDeployments silently overwrites with the last one seen. The test verifies it picks deploy-789 over deploy-456 (stopped), but if two RUNNING deployments existed for the same model the behavior is undefined (last-write-wins from iteration order). A log warning or first-wins strategy would be more predictable, though this is unlikely in practice.
[NIT] The field name http shadows the standard library package net/http. While this works because the package is only referenced by its qualified name within the struct methods, renaming to httpClient would avoid the potential confusion and better align with the pattern used elsewhere (e.g. c.http.Do).