Skip to content

feat(golangGRPCgolang gRPC client code package - #873

Closed
zetxqx wants to merge 1 commit into
smg-project:mainfrom
zetxqx:goclient
Closed

zetxqx wants to merge 1 commit into
smg-project:mainfrom
zetxqx:goclient

Conversation

@zetxqx

@zetxqx zetxqx commented Mar 24, 2026 •

Copy link
Copy Markdown
Contributor

Description

Problem

The project currently provides Rust and Python clients for ModelServers defined in crates/grpc_client/proto, but lacks a Go client support. Go consumers cannot easily import and use the services without manual boilerplate setups.

Solution

Introduced a dedicated Go module framework at crates/grpc_client/go with automated code generation and CI release workflows. Specifically:

  • Generation Configuration: Added a generate.sh automating protoc generation.
  • Release Pipeline: Introduced GitHub Action workflow .github/workflows/release-go.yml that runs verification tests on branches and automates releasing versioned Git tags (e.g., crates/grpc_client/go/vX.Y.Z) carrying the forced-add generated outputs for external consumers to safely fetch.

Changes

  • .github/workflows/release-go.yml: Workflow to test package building & trigger automatic Git tags/release branches push
  • crates/grpc_client/go/generate.sh: Shell-script triggers for individual service protobuf to gRPC structure compilation.
  • crates/grpc_client/go/VERSION: Basic semantic track definition point for GO.
  • crates/grpc_client/go/generate.go: Short wrapper support for continuous go:generate bindings support.
  • crates/grpc_client/go/{go.mod, go.sum}: Go modules initial descriptors setup for dependencies referencing.

Test Plan

Run the following command locally

cd crates/grpc_client/go
bash generate.sh
Checklist
  • cargo +nightly fmt passes
  • cargo clippy --all-targets --all-features -- -D warnings passes
  • (Optional) Documentation updated
  • (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

Summary by CodeRabbit

  • New Features

    • Launched Go gRPC client package (v0.1.0) for Shepherd Model Gateway services, available via go get.
    • Includes pre-generated gRPC client code for encoder, scheduler, and LLM service integrations.
  • Chores

    • Added automated CI/CD workflow for testing and releasing Go client packages.
    • Added documentation for package setup and code regeneration from protobuf definitions.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@github-actions github-actions Bot added documentation Improvements or additions to documentation ci CI/CD configuration changes labels Mar 24, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request establishes a comprehensive Go module for gRPC client interactions with ModelServers, filling a critical gap in client language support. It streamlines the development workflow by automating the generation of Go gRPC code from protobuf definitions and ensures reliable distribution through an integrated CI/CD release pipeline that creates versioned Git tags.

Highlights

  • New Go gRPC Client Module: A dedicated Go module framework has been introduced at crates/grpc_client/go to provide Go client support for ModelServers, addressing the current lack of Go client capabilities.
  • Automated Code Generation: A generate.sh script and generate.go wrapper have been added to automate the protoc generation process, compiling individual service protobufs into Go gRPC structures.
  • CI Release Workflow: A new GitHub Actions workflow (.github/workflows/release-go.yml) has been implemented to automate the release process, including verification tests and the creation of versioned Git tags (e.g., crates/grpc_client/go/vX.Y.Z) for external consumption.
Ignored Files
  • Ignored by pattern: .github/workflows/** (1)
    • .github/workflows/release-go.yml
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@coderabbitai

coderabbitai Bot commented Mar 24, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Introduces a complete Go gRPC client generation and release infrastructure, including a GitHub Actions workflow for testing and automated tagging, Go module dependencies, a Bash script for protobuf code generation with protoc, code generation driver in Go, project configuration files, and documentation for the auto-generated client library.

Changes

Cohort / File(s) Summary
CI/Release Automation
.github/workflows/release-go.yml
Adds GitHub Actions workflow to test Go code, run code generation, and create git tags for releases; includes job dependencies, tag existence checks, and version file reading.
Go Module Setup
crates/grpc_client/go/go.mod
Defines Go module path, Go version 1.25.0, and dependencies on gRPC and protobuf libraries with indirect transitive dependencies.
Code Generation
crates/grpc_client/go/generate.sh, crates/grpc_client/go/generate.go
Shell script that locates protoc plugins, creates output directories, copies proto files, and runs protoc with Go/gRPC generators and import path mappings; Go directive triggers script execution.
Project Configuration
crates/grpc_client/go/.gitignore, crates/grpc_client/go/VERSION
Ignores generated directories and proto working files; specifies semantic version for release tagging.
Documentation
crates/grpc_client/go/README.md
User-facing guide describing module purpose, installation, directory layout, code regeneration steps, and release/tag consumption instructions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested labels

grpc, dependencies

Suggested reviewers

  • key4ng
  • slin1237
  • CatherineSue
  • XinyueZhang369

Poem

🐰✨ A bunny hops through Go proto files,
With protoc magic running for miles!
Generated code flows like spring grass,
Tags and releases—a smooth, swift pass! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title is incomplete and malformed—it reads 'feat(golangGRPCgolang gRPC client code package' with missing closing parenthesis and lacks clarity on the primary change. Revise the title to be grammatically correct and clearly summarize the main change, e.g., 'feat: Add Go gRPC client code generation workflow and module' or similar.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: bobzetian <bobzetian@google.com>
@mergify

mergify Bot commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Hi @zetxqx, the DCO sign-off check has failed. All commits must include a Signed-off-by line.

To fix existing commits:

# Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-lease

To sign off future commits automatically:

  • Use git commit -s every time, or
  • VSCode: enable Git: Always Sign Off in Settings
  • PyCharm: enable Sign-off commit in the Commit tool window

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

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.

Code Review

This review addresses the new Go gRPC client package. It highlights a critical issue with invalid Go and dependency versions in go.mod, suggests refactoring the repetitive code generation script for better maintainability, and recommends an idiomatic package name for the Go source file. Addressing these points will ensure the new Go client is robust, maintainable, and usable.

Comment on lines +1 to +15
module github.com/lightseek/smg/crates/grpc_client/go

go 1.25.0

require (
google.golang.org/grpc v1.79.3
google.golang.org/protobuf v1.36.11
)

require (
golang.org/x/net v0.48.0 // indirect
golang.org/x/sys v0.39.0 // indirect
golang.org/x/text v0.32.0 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20251202230838-ff82c1b0f217 // indirect
)

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 go.mod file contains invalid versions for Go and its dependencies. This will prevent the module from being built or used.

  • The Go version 1.25.0 is not a valid Go version. The latest stable version is 1.22.x. Please use a valid and preferably widely-adopted version.
  • The dependency versions also appear to be invalid or from the future (e.g., google.golang.org/grpc v1.79.3 when the latest is v1.64.0, google.golang.org/genproto/... v0.0.0-2025...).

Please run go mod tidy to clean up the go.mod and go.sum files with correct, existing versions of the dependencies.

References
  1. Ensure that all external resource versions specified in scripts (or configuration files like go.mod that dictate build processes) are accurate and exist, as incorrect versions can lead to build failures.

Comment on lines +1 to +3
// Package go_client is used to trigger code generation.
//go:generate bash ./generate.sh
package go_client

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.

medium

The package name go_client is not idiomatic. Go package names should be short, concise, all lowercase, and should not use underscores. Please consider renaming it to goclient for better adherence to Go conventions.

Suggested change
// Package go_client is used to trigger code generation.
//go:generate bash ./generate.sh
package go_client
// Package goclient is used to trigger code generation.
//go:generate bash ./generate.sh
package goclient

Comment on lines +54 to +112
# SGLang Encoder
protoc \
--plugin=protoc-gen-go="$PROTOC_GEN_GO" \
--plugin=protoc-gen-go-grpc="$PROTOC_GEN_GO_GRPC" \
--proto_path=. \
--go_out="$OUTPUT_DIR/sglang_encoder" \
--go_opt=paths=source_relative \
$MAPPINGS \
--go_opt=Msglang_encoder.proto=github.com/lightseek/smg/crates/grpc_client/go/generated/sglang_encoder \
--go-grpc_out="$OUTPUT_DIR/sglang_encoder" \
--go-grpc_opt=paths=source_relative \
$MAPPINGS_GRPC \
--go-grpc_opt=Msglang_encoder.proto=github.com/lightseek/smg/crates/grpc_client/go/generated/sglang_encoder \
sglang_encoder.proto

# SGLang Scheduler
protoc \
--plugin=protoc-gen-go="$PROTOC_GEN_GO" \
--plugin=protoc-gen-go-grpc="$PROTOC_GEN_GO_GRPC" \
--proto_path=. \
--go_out="$OUTPUT_DIR/sglang_scheduler" \
--go_opt=paths=source_relative \
$MAPPINGS \
--go_opt=Msglang_scheduler.proto=github.com/lightseek/smg/crates/grpc_client/go/generated/sglang_scheduler \
--go-grpc_out="$OUTPUT_DIR/sglang_scheduler" \
--go-grpc_opt=paths=source_relative \
$MAPPINGS_GRPC \
--go-grpc_opt=Msglang_scheduler.proto=github.com/lightseek/smg/crates/grpc_client/go/generated/sglang_scheduler \
sglang_scheduler.proto

# TRT-LLM Service
protoc \
--plugin=protoc-gen-go="$PROTOC_GEN_GO" \
--plugin=protoc-gen-go-grpc="$PROTOC_GEN_GO_GRPC" \
--proto_path=. \
--go_out="$OUTPUT_DIR/trtllm" \
--go_opt=paths=source_relative \
$MAPPINGS \
--go_opt=Mtrtllm_service.proto=github.com/lightseek/smg/crates/grpc_client/go/generated/trtllm \
--go-grpc_out="$OUTPUT_DIR/trtllm" \
--go-grpc_opt=paths=source_relative \
$MAPPINGS_GRPC \
--go-grpc_opt=Mtrtllm_service.proto=github.com/lightseek/smg/crates/grpc_client/go/generated/trtllm \
trtllm_service.proto

# vLLM Engine
protoc \
--plugin=protoc-gen-go="$PROTOC_GEN_GO" \
--plugin=protoc-gen-go-grpc="$PROTOC_GEN_GO_GRPC" \
--proto_path=. \
--go_out="$OUTPUT_DIR/vllm" \
--go_opt=paths=source_relative \
$MAPPINGS \
--go_opt=Mvllm_engine.proto=github.com/lightseek/smg/crates/grpc_client/go/generated/vllm \
--go-grpc_out="$OUTPUT_DIR/vllm" \
--go-grpc_opt=paths=source_relative \
$MAPPINGS_GRPC \
--go-grpc_opt=Mvllm_engine.proto=github.com/lightseek/smg/crates/grpc_client/go/generated/vllm \
vllm_engine.proto

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.

medium

The protoc commands for generating code for each service (SGLang Encoder, SGLang Scheduler, TRT-LLM Service, vLLM Engine) are very similar and repetitive. This makes the script harder to maintain. Consider refactoring this section into a loop or a function to reduce duplication.

For example, you could define a function:

generate_service() {
    local service_name="$1"
    local proto_file="$2"
    local output_dir="$OUTPUT_DIR/$service_name"

    echo "Generating for $service_name..."

    protoc \
      --plugin=protoc-gen-go="$PROTOC_GEN_GO" \
      --plugin=protoc-gen-go-grpc="$PROTOC_GEN_GO_GRPC" \
      --proto_path=. \
      --go_out="$output_dir" \
      --go_opt=paths=source_relative \
      $MAPPINGS \
      --go_opt=M${proto_file}=github.com/lightseek/smg/crates/grpc_client/go/generated/${service_name} \
      --go-grpc_out="$output_dir" \
      --go-grpc_opt=paths=source_relative \
      $MAPPINGS_GRPC \
      --go-grpc_opt=M${proto_file}=github.com/lightseek/smg/crates/grpc_client/go/generated/${service_name} \
      "$proto_file"
}

And then call it for each service:

generate_service "sglang_encoder" "sglang_encoder.proto"
generate_service "sglang_scheduler" "sglang_scheduler.proto"
generate_service "trtllm" "trtllm_service.proto"
generate_service "vllm" "vllm_engine.proto"

This would make the script cleaner and easier to extend with new services.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/release-go.yml:
- Around line 77-87: The workflow commits generated code in the "Commit
Generated Code" step but never pushes that commit before creating the release
tag; update the step to push the committed changes (git push origin
HEAD:<branch> or equivalent) before the tag creation step (and/or ensure the tag
is created from the remote branch), or alternatively create and push the tag
that points to the new commit (push the commit first then push the tag); modify
the step that performs the commit so it runs a push when a commit was created
and ensure downstream steps that create tags use the pushed commit (reference
the "Commit Generated Code" step and the tag creation step when making the
change).
- Around line 89-97: In the "Read Version" step the VERSION variable should be
safely quoted to avoid word-splitting; update the write to GitHub Actions output
to use a quoted expansion and file target (e.g., emit "version=$VERSION" using a
quoted variable and quote $GITHUB_OUTPUT as well) so change the current
echo/write that references VERSION and GITHUB_OUTPUT in the step with id
"version" to use quoted expansions.
- Around line 99-107: The TAG variable in the check_tag step is not quoted when
used; update the git rev-parse and echo lines to quote "$TAG" (and any other
expansions) so the command and outputs handle tags with spaces or special chars
correctly—specifically change uses of git rev-parse $TAG to git rev-parse "$TAG"
and echo "exists=true" >> $GITHUB_OUTPUT (if echoing TAG include "$TAG") in the
step with id check_tag where TAG="crates/grpc_client/go/v${{
steps.version.outputs.version }}".

In `@crates/grpc_client/go/generate.go`:
- Around line 1-3: The package declaration uses a non-idiomatic name
`go_client`; change the package name to `goclient` by updating the `package
go_client` declaration in this file and any other files in the same package to
`package goclient`, then update all import sites that reference `go_client` to
import `goclient` instead (and run go build or `go vet` to find remaining
references); ensure the `//go:generate` directive and any generated artifacts
still work with the new package name.

In `@crates/grpc_client/go/go.mod`:
- Line 3: Update the Go version declaration in go.mod from "go 1.25.0" to the
desired newer stable version (e.g., "go 1.26.1") by changing the module's go
directive so the line reading go 1.25.0 is replaced with go 1.26.1; ensure the
go.mod's go directive matches the target Go toolchain you intend to use.
- Around line 1-8: The repo currently generates duplicate Go bindings for
sglang_scheduler.proto into generated/sglang_scheduler (import path
github.com/lightseek/smg/crates/grpc_client/go/generated/sglang_scheduler) while
bindings/golang emits the same types into internal/proto
(github.com/lightseek/smg/go-grpc-sdk/internal/proto), causing import conflicts
for consumers; fix by choosing one of three options and applying it: (A) stop
generating sglang_scheduler in crates/grpc_client/go (remove that proto from its
generator config) and add a short README in crates/grpc_client/go explaining
consumers should use bindings/golang for sglang_scheduler, (B) change the
generated package/module path for crates/grpc_client/go (or its output folder)
to a distinct import path to avoid collision, or (C) limit each generator to a
non‑overlapping subset of services (e.g., only low‑level bindings in one and SDK
wrappers in the other) and document the intended consumer preference in both
module READMEs; update generator configs and add the documentation accordingly
so imports no longer collide.

In `@crates/grpc_client/go/README.md`:
- Around line 27-35: The markdown fenced code blocks in the numbered list (the
two ```bash blocks) violate MD031 because they lack blank lines before and after
the fences; edit the README.md content around those code blocks so there is an
empty line immediately before each opening ```bash and an empty line immediately
after each closing ``` to separate the code blocks from the surrounding list
text.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: dd34f854-2969-4af3-9f3e-e9bb66359508

📥 Commits

Reviewing files that changed from the base of the PR and between 8a463a1 and 5618f48.

⛔ Files ignored due to path filters (1)
  • crates/grpc_client/go/go.sum is excluded by !**/*.sum
📒 Files selected for processing (7)
  • .github/workflows/release-go.yml
  • crates/grpc_client/go/.gitignore
  • crates/grpc_client/go/README.md
  • crates/grpc_client/go/VERSION
  • crates/grpc_client/go/generate.go
  • crates/grpc_client/go/generate.sh
  • crates/grpc_client/go/go.mod

Comment on lines +77 to +87
- name: Commit Generated Code
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
git add -f crates/grpc_client/go/generated
if [ -d "crates/grpc_client/go/proto" ]; then
git add -f crates/grpc_client/go/proto
fi
if ! git diff --cached --quiet; then
git commit -m "chore: include generated code for release [skip ci]"
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Release commit is created but not pushed before tagging.

The workflow commits generated code locally but doesn't push the commit before creating the tag. The tag will be created on the local commit (with generated code), but this commit won't be pushed to the main branch. This means:

  1. The tag points to a commit that doesn't exist on main
  2. Consumers fetching the tag will get the generated code, but the commit history diverges from main

If this is intentional (tags are orphaned commits with generated code), consider documenting this behavior. Otherwise, you may want to push to a release branch or reconsider the release strategy.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/release-go.yml around lines 77 - 87, The workflow commits
generated code in the "Commit Generated Code" step but never pushes that commit
before creating the release tag; update the step to push the committed changes
(git push origin HEAD:<branch> or equivalent) before the tag creation step
(and/or ensure the tag is created from the remote branch), or alternatively
create and push the tag that points to the new commit (push the commit first
then push the tag); modify the step that performs the commit so it runs a push
when a commit was created and ensure downstream steps that create tags use the
pushed commit (reference the "Commit Generated Code" step and the tag creation
step when making the change).

Comment on lines +89 to +97
- name: Read Version
id: version
run: |
if [ ! -f crates/grpc_client/go/VERSION ]; then
echo "Error: VERSION file missing"
exit 1
fi
VERSION=$(cat crates/grpc_client/go/VERSION | tr -d '[:space:]')
echo "version=$VERSION" >> $GITHUB_OUTPUT

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Quote variable to prevent word splitting (SC2086).

The $VERSION variable should be quoted to prevent potential issues with word splitting, even though tr -d '[:space:]' should have removed whitespace.

♻️ Suggested fix
       - name: Read Version
         id: version
         run: |
           if [ ! -f crates/grpc_client/go/VERSION ]; then
             echo "Error: VERSION file missing"
             exit 1
           fi
-          VERSION=$(cat crates/grpc_client/go/VERSION | tr -d '[:space:]')
+          VERSION="$(cat crates/grpc_client/go/VERSION | tr -d '[:space:]')"
           echo "version=$VERSION" >> $GITHUB_OUTPUT
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Read Version
id: version
run: |
if [ ! -f crates/grpc_client/go/VERSION ]; then
echo "Error: VERSION file missing"
exit 1
fi
VERSION=$(cat crates/grpc_client/go/VERSION | tr -d '[:space:]')
echo "version=$VERSION" >> $GITHUB_OUTPUT
- name: Read Version
id: version
run: |
if [ ! -f crates/grpc_client/go/VERSION ]; then
echo "Error: VERSION file missing"
exit 1
fi
VERSION="$(cat crates/grpc_client/go/VERSION | tr -d '[:space:]')"
echo "version=$VERSION" >> $GITHUB_OUTPUT
🧰 Tools
🪛 actionlint (1.7.11)

[error] 91-91: shellcheck reported issue in this script: SC2086:info:6:28: Double quote to prevent globbing and word splitting

(shellcheck)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/release-go.yml around lines 89 - 97, In the "Read Version"
step the VERSION variable should be safely quoted to avoid word-splitting;
update the write to GitHub Actions output to use a quoted expansion and file
target (e.g., emit "version=$VERSION" using a quoted variable and quote
$GITHUB_OUTPUT as well) so change the current echo/write that references VERSION
and GITHUB_OUTPUT in the step with id "version" to use quoted expansions.

Comment on lines +99 to +107
- name: Check if tag exists
id: check_tag
run: |
TAG="crates/grpc_client/go/v${{ steps.version.outputs.version }}"
if git rev-parse "$TAG" >/dev/null 2>&1; then
echo "exists=true" >> $GITHUB_OUTPUT
else
echo "exists=false" >> $GITHUB_OUTPUT
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Quote variable expansion in tag check (SC2086).

The $TAG variable should be quoted in the git rev-parse command and output statements.

♻️ Suggested fix
       - name: Check if tag exists
         id: check_tag
         run: |
-          TAG="crates/grpc_client/go/v${{ steps.version.outputs.version }}"
+          TAG="crates/grpc_client/go/v${{ steps.version.outputs.version }}"
           if git rev-parse "$TAG" >/dev/null 2>&1; then
-            echo "exists=true" >> $GITHUB_OUTPUT
+            echo "exists=true" >> "$GITHUB_OUTPUT"
           else
-            echo "exists=false" >> $GITHUB_OUTPUT
+            echo "exists=false" >> "$GITHUB_OUTPUT"
           fi
🧰 Tools
🪛 actionlint (1.7.11)

[error] 101-101: shellcheck reported issue in this script: SC2086:info:3:25: Double quote to prevent globbing and word splitting

(shellcheck)


[error] 101-101: shellcheck reported issue in this script: SC2086:info:5:26: Double quote to prevent globbing and word splitting

(shellcheck)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/release-go.yml around lines 99 - 107, The TAG variable in
the check_tag step is not quoted when used; update the git rev-parse and echo
lines to quote "$TAG" (and any other expansions) so the command and outputs
handle tags with spaces or special chars correctly—specifically change uses of
git rev-parse $TAG to git rev-parse "$TAG" and echo "exists=true" >>
$GITHUB_OUTPUT (if echoing TAG include "$TAG") in the step with id check_tag
where TAG="crates/grpc_client/go/v${{ steps.version.outputs.version }}".

Comment on lines +1 to +3
// Package go_client is used to trigger code generation.
//go:generate bash ./generate.sh
package go_client

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Non-idiomatic Go package name.

Go package names should be lowercase without underscores. Consider renaming go_client to goclient.

♻️ Suggested fix
-// Package go_client is used to trigger code generation.
+// Package goclient is used to trigger code generation.
 //go:generate bash ./generate.sh
-package go_client
+package goclient
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Package go_client is used to trigger code generation.
//go:generate bash ./generate.sh
package go_client
// Package goclient is used to trigger code generation.
//go:generate bash ./generate.sh
package goclient
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/grpc_client/go/generate.go` around lines 1 - 3, The package
declaration uses a non-idiomatic name `go_client`; change the package name to
`goclient` by updating the `package go_client` declaration in this file and any
other files in the same package to `package goclient`, then update all import
sites that reference `go_client` to import `goclient` instead (and run go build
or `go vet` to find remaining references); ensure the `//go:generate` directive
and any generated artifacts still work with the new package name.

Comment on lines +1 to +8
module github.com/lightseek/smg/crates/grpc_client/go

go 1.25.0

require (
google.golang.org/grpc v1.79.3
google.golang.org/protobuf v1.36.11
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if bindings/golang module exists and what it provides
echo "=== Checking existing Go module at bindings/golang ==="
if [ -f "bindings/golang/go.mod" ]; then
  cat bindings/golang/go.mod
  echo ""
  echo "=== Directory structure ==="
  tree bindings/golang -L 3 2>/dev/null || find bindings/golang -type f -name "*.go" | head -20
fi

Repository: lightseekorg/smg

Length of output: 1788


🏁 Script executed:

#!/bin/bash
# Check if crates/grpc_client/go exists and what it contains
echo "=== Checking crates/grpc_client/go ==="
if [ -d "crates/grpc_client/go" ]; then
  echo "Directory exists. Contents:"
  find crates/grpc_client/go -type f -name "*.pb.go" -o -name "*.proto" | head -30
  echo ""
  echo "=== Directory tree ==="
  tree crates/grpc_client/go -L 3 2>/dev/null || find crates/grpc_client/go -type f | head -30
else
  echo "Directory does not exist"
fi

echo ""
echo "=== Checking what protos are used by bindings/golang ==="
# Check if there are other proto generated files in bindings/golang
find bindings/golang -type f \( -name "*.pb.go" -o -name "*.proto" \) 2>/dev/null

echo ""
echo "=== Checking grpc_client directory structure ==="
# Check what's in the grpc_client directory
if [ -d "crates/grpc_client" ]; then
  find crates/grpc_client -maxdepth 2 -type d
fi

Repository: lightseekorg/smg

Length of output: 651


🏁 Script executed:

#!/bin/bash
# Check what the generate scripts do
echo "=== generate.sh ==="
cat crates/grpc_client/go/generate.sh

echo ""
echo "=== generate.go ==="
cat crates/grpc_client/go/generate.go

echo ""
echo "=== README.md ==="
cat crates/grpc_client/go/README.md

Repository: lightseekorg/smg

Length of output: 5626


Duplicate sglang_scheduler generation with different import paths.

This module generates Go code for sglang_scheduler.proto to generated/sglang_scheduler/ with import path github.com/lightseek/smg/crates/grpc_client/go/generated/sglang_scheduler, while the existing bindings/golang module generates the same proto to internal/proto/ with path github.com/lightseek/smg/go-grpc-sdk/internal/proto. Consumers importing both modules will face import conflicts.

While these modules serve different purposes (raw auto-generated bindings vs. higher-level wrapper SDK), consider documenting which module consumers should prefer, or ensuring the two modules target different subsets of services to avoid overlap.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/grpc_client/go/go.mod` around lines 1 - 8, The repo currently
generates duplicate Go bindings for sglang_scheduler.proto into
generated/sglang_scheduler (import path
github.com/lightseek/smg/crates/grpc_client/go/generated/sglang_scheduler) while
bindings/golang emits the same types into internal/proto
(github.com/lightseek/smg/go-grpc-sdk/internal/proto), causing import conflicts
for consumers; fix by choosing one of three options and applying it: (A) stop
generating sglang_scheduler in crates/grpc_client/go (remove that proto from its
generator config) and add a short README in crates/grpc_client/go explaining
consumers should use bindings/golang for sglang_scheduler, (B) change the
generated package/module path for crates/grpc_client/go (or its output folder)
to a distinct import path to avoid collision, or (C) limit each generator to a
non‑overlapping subset of services (e.g., only low‑level bindings in one and SDK
wrappers in the other) and document the intended consumer preference in both
module READMEs; update generator configs and add the documentation accordingly
so imports no longer collide.

@@ -0,0 +1,15 @@
module github.com/lightseek/smg/crates/grpc_client/go

go 1.25.0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🌐 Web query:

What is the latest stable version of Go programming language?

💡 Result:

The latest stable version of the Go programming language is 1.26.1.

Citations:


Go 1.25.0 is a released version. It is stable and supported. If desired, consider updating to Go 1.26.1, the current latest stable version.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/grpc_client/go/go.mod` at line 3, Update the Go version declaration in
go.mod from "go 1.25.0" to the desired newer stable version (e.g., "go 1.26.1")
by changing the module's go directive so the line reading go 1.25.0 is replaced
with go 1.26.1; ensure the go.mod's go directive matches the target Go toolchain
you intend to use.

Comment on lines +27 to +35
1. Install prerequisites:
```bash
go install google.golang.org/protobuf/cmd/protoc-gen-go@latest
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
```
2. Run the generation script:
```bash
bash generate.sh
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Add blank lines around fenced code blocks for markdown compliance.

The fenced code blocks within the numbered list items should be surrounded by blank lines per markdown standards (MD031).

♻️ Suggested fix
 1.  Install prerequisites:
+
     ```bash
     go install google.golang.org/protobuf/cmd/protoc-gen-go@latest
     go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
     ```
+
 2.  Run the generation script:
+
     ```bash
     bash generate.sh
     ```
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
1. Install prerequisites:
```bash
go install google.golang.org/protobuf/cmd/protoc-gen-go@latest
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
```
2. Run the generation script:
```bash
bash generate.sh
```
1. Install prerequisites:
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 28-28: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


[warning] 31-31: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


[warning] 33-33: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/grpc_client/go/README.md` around lines 27 - 35, The markdown fenced
code blocks in the numbered list (the two ```bash blocks) violate MD031 because
they lack blank lines before and after the fences; edit the README.md content
around those code blocks so there is an empty line immediately before each
opening ```bash and an empty line immediately after each closing ``` to separate
the code blocks from the surrounding list text.

@zetxqx zetxqx closed this Mar 24, 2026
@zetxqx
zetxqx deleted the goclient branch March 24, 2026 00:34
ConnorLi96 added a commit that referenced this pull request Apr 22, 2026
…cs endpoint

Option A (sentinel round-trip): encode ':' to '__smgcolon48f__' before
passing text to openmetrics_parser (which rejects colons), then decode
the sentinel back to ':' after serialization. This preserves colons in
both metric names (sglang:num_running_reqs) and label values
(grpc://host:9001).

Option A chosen over B (passthrough-when-no-labels) because labels are
always injected by get_engine_metrics, making B a no-op. Option C
(raw text injection) was rejected as the largest diff with full
Prometheus syntax responsibility.

The sentinel uses only lowercase to avoid conflicts with
openmetrics_parser's case-sensitive keyword grammar (HELP/TYPE).

Before: sglang_num_running_reqs (colon clobbered to underscore at
metrics_aggregator.rs:19 via .replace(":", "_"))
After:  sglang:num_running_reqs (original Prometheus namespace:name
format preserved)

Tracks: consolidation-doc §3.A-ish — colon preservation follow-up to #873
Signed-off-by: Connor Li <ConnorLi96@users.noreply.github.com>
ConnorLi96 added a commit that referenced this pull request Apr 22, 2026
…cs endpoint

Option A (sentinel round-trip): encode ':' to '__smgcolon48f__' before
passing text to openmetrics_parser (which rejects colons), then decode
the sentinel back to ':' after serialization. This preserves colons in
both metric names (sglang:num_running_reqs) and label values
(grpc://host:9001).

Option A chosen over B (passthrough-when-no-labels) because labels are
always injected by get_engine_metrics, making B a no-op. Option C
(raw text injection) was rejected as the largest diff with full
Prometheus syntax responsibility.

The sentinel uses only lowercase to avoid conflicts with
openmetrics_parser's case-sensitive keyword grammar (HELP/TYPE).

Before: sglang_num_running_reqs (colon clobbered to underscore at
metrics_aggregator.rs:19 via .replace(":", "_"))
After:  sglang:num_running_reqs (original Prometheus namespace:name
format preserved)

Tracks: consolidation-doc §3.A-ish — colon preservation follow-up to #873
Signed-off-by: Connor Li <ConnorLi96@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI/CD configuration changes documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant