fix: address all review findings (zero remaining)
CI / test (pull_request) Successful in 13s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m43s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 2m19s

Tests:
- Add WithTemperature tests (builder method, chaining, zero omission)
- Add temperature serialization tests (omitted when 0, included when set)

Composite action:
- Use python3 for robust JSON version parsing (replaces sed)
- Verify SHA-256 checksum before executing downloaded binary
- Wire up repo input (no longer hardcodes rodin/review-bot)

Release workflow:
- Handle 409 conflict (existing release for tag)
- Use file-based JSON parsing for reliability

Code:
- Tighten WithTemperature doc comment (single clear line)
- Fix flag alignment (missing tab on llmTemp declaration)
This commit is contained in:
Rodin
2026-05-01 11:58:21 -07:00
parent 59fbd38837
commit 46c63ed121
5 changed files with 128 additions and 17 deletions
+29 -3
View File
@@ -58,8 +58,10 @@ runs:
shell: bash
run: |
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
if [ "${{ inputs.version }}" = "latest" ]; then
VERSION=$(curl -sSf "${GITEA_URL}/api/v1/repos/rodin/review-bot/releases?limit=1" | sed -n 's/.*"tag_name":"\([^"]*\)".*/\1/p' | head -1)
VERSION=$(curl -sSf "${GITEA_URL}/api/v1/repos/${REPO}/releases?limit=1" \
| python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
if [ -z "$VERSION" ]; then
echo "Failed to determine latest version" >&2
exit 1
@@ -81,9 +83,33 @@ runs:
shell: bash
run: |
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
VERSION="${{ steps.version.outputs.version }}"
curl -sSfL "${GITEA_URL}/rodin/review-bot/releases/download/${VERSION}/review-bot-linux-amd64" -o ${{ runner.temp }}/review-bot
chmod +x ${{ runner.temp }}/review-bot
BINARY="review-bot-linux-amd64"
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/${BINARY}" \
-o "${{ runner.temp }}/review-bot"
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/checksums.txt" \
-o "${{ runner.temp }}/checksums.txt"
# Verify SHA-256 checksum
cd "${{ runner.temp }}"
EXPECTED=$(grep "${BINARY}" checksums.txt | awk '{print $1}')
ACTUAL=$(sha256sum review-bot | awk '{print $1}')
if [ -z "$EXPECTED" ]; then
echo "Error: no checksum found for ${BINARY}" >&2
exit 1
fi
if [ "$EXPECTED" != "$ACTUAL" ]; then
echo "Error: checksum mismatch!" >&2
echo " Expected: $EXPECTED" >&2
echo " Actual: $ACTUAL" >&2
exit 1
fi
chmod +x "${{ runner.temp }}/review-bot"
echo "Installed review-bot ${VERSION} (checksum verified)"
- name: Run review
shell: bash
+20 -9
View File
@@ -38,23 +38,34 @@ jobs:
GITEA_URL="${{ github.server_url }}"
REPO="${{ github.repository }}"
# Create release
RESPONSE=$(curl -sSf -X POST \
# Create release (or find existing one for this tag)
HTTP_CODE=$(curl -s -o /tmp/release_response.json -w "%{http_code}" -X POST \
-H "Authorization: token ${GITEA_TOKEN}" \
-H "Content-Type: application/json" \
"${GITEA_URL}/api/v1/repos/${REPO}/releases" \
-d "{\"tag_name\": \"${VERSION}\", \"name\": \"${VERSION}\", \"body\": \"Release ${VERSION}\", \"draft\": false, \"prerelease\": false}")
# Parse release ID using Python (robust JSON parsing)
RELEASE_ID=$(echo "$RESPONSE" | python3 -c "import sys, json; print(json.load(sys.stdin)['id'])")
if [ -z "$RELEASE_ID" ]; then
echo "Failed to create release" >&2
echo "$RESPONSE" >&2
if [ "$HTTP_CODE" = "409" ]; then
echo "Release for ${VERSION} already exists, fetching existing..."
curl -sSf -o /tmp/release_response.json \
-H "Authorization: token ${GITEA_TOKEN}" \
"${GITEA_URL}/api/v1/repos/${REPO}/releases/tags/${VERSION}"
elif [ "$HTTP_CODE" != "201" ]; then
echo "Failed to create release (HTTP ${HTTP_CODE})" >&2
cat /tmp/release_response.json >&2
exit 1
fi
echo "Created release ID: ${RELEASE_ID}"
# Parse release ID (python3 available on ubuntu-24.04 runners)
RELEASE_ID=$(python3 -c "import json; print(json.load(open('/tmp/release_response.json'))['id'])")
if [ -z "$RELEASE_ID" ]; then
echo "Failed to parse release ID" >&2
cat /tmp/release_response.json >&2
exit 1
fi
echo "Release ID: ${RELEASE_ID}"
# Upload each asset
for file in dist/*; do
+1 -1
View File
@@ -28,7 +28,7 @@ func main() {
llmModel := flag.String("llm-model", envOrDefault("LLM_MODEL", ""), "LLM model name")
conventionsFile := flag.String("conventions-file", envOrDefault("CONVENTIONS_FILE", ""), "Conventions file path in repo (e.g. CLAUDE.md)")
dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting")
llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)")
llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)")
flag.Parse()
+1 -4
View File
@@ -28,10 +28,7 @@ func NewClient(baseURL, apiKey, model string) *Client {
}
}
// WithTemperature sets the temperature for LLM requests.
// A value of 0 (the zero value) means the field is omitted from the request,
// causing the server to use its default temperature.
// If not set (zero value), the server default is used.
// WithTemperature sets the temperature for LLM requests (0 = omit, uses server default).
func (c *Client) WithTemperature(t float64) *Client {
c.Temperature = t
return c
+77
View File
@@ -108,3 +108,80 @@ func TestComplete_ServerDown(t *testing.T) {
t.Fatal("expected error for connection refused, got nil")
}
}
func TestWithTemperature(t *testing.T) {
client := NewClient("http://example.com", "key", "model")
if client.Temperature != 0 {
t.Errorf("expected initial temperature 0, got %f", client.Temperature)
}
result := client.WithTemperature(0.7)
if result != client {
t.Error("WithTemperature should return the same client for chaining")
}
if client.Temperature != 0.7 {
t.Errorf("expected temperature 0.7, got %f", client.Temperature)
}
}
func TestComplete_TemperatureOmittedWhenZero(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var req map[string]interface{}
json.NewDecoder(r.Body).Decode(&req)
if _, exists := req["temperature"]; exists {
t.Error("temperature should be omitted when zero (server default)")
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(ChatResponse{
Choices: []struct {
Message struct {
Content string `json:"content"`
} `json:"message"`
}{{Message: struct {
Content string `json:"content"`
}{Content: "ok"}}},
})
}))
defer server.Close()
client := NewClient(server.URL, "key", "model")
_, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
func TestComplete_TemperatureIncludedWhenSet(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var req map[string]interface{}
json.NewDecoder(r.Body).Decode(&req)
temp, exists := req["temperature"]
if !exists {
t.Error("temperature should be included when set")
}
if temp != 0.7 {
t.Errorf("expected temperature 0.7, got %v", temp)
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(ChatResponse{
Choices: []struct {
Message struct {
Content string `json:"content"`
} `json:"message"`
}{{Message: struct {
Content string `json:"content"`
}{Content: "ok"}}},
})
}))
defer server.Close()
client := NewClient(server.URL, "key", "model").WithTemperature(0.7)
_, err := client.Complete([]Message{{Role: "user", Content: "Hi"}})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}