-
Notifications
You must be signed in to change notification settings - Fork 142
feat(router): add cache_aware_length policy with long/short pool split #2252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,211 @@ | ||||||||||||
| name: Build Docker Images | ||||||||||||
|
|
||||||||||||
| on: | ||||||||||||
| push: | ||||||||||||
| branches: ["main", "test"] | ||||||||||||
| workflow_dispatch: | ||||||||||||
|
|
||||||||||||
| env: | ||||||||||||
| REGISTRY: ghcr.io | ||||||||||||
| IMAGE_PREFIX: ${{ github.repository_owner }}/1xtoken | ||||||||||||
| CLUSTER_GOPROXY: https://proxy.golang.org | ||||||||||||
| # 阿里云镜像仓库:国内服务器拉 ghcr.io 太慢,构建时同步推一份到阿里云 | ||||||||||||
| ALIYUN_REGISTRY: crpi-w0utadukedw6a9ld.cn-hangzhou.personal.cr.aliyuncs.com | ||||||||||||
| ALIYUN_NAMESPACE: 1xtoken | ||||||||||||
|
|
||||||||||||
| jobs: | ||||||||||||
| # ------------------------------------------------------------------ | ||||||||||||
| # 阶段 1: 智能分析 (Configuration Job) | ||||||||||||
| # ------------------------------------------------------------------ | ||||||||||||
| configure: | ||||||||||||
| name: Configure Matrix | ||||||||||||
| runs-on: astro-web3-arc-runner-set | ||||||||||||
| outputs: | ||||||||||||
| has-changes: ${{ steps.detect.outputs.has-changes }} | ||||||||||||
| matrix: ${{ steps.matrix.outputs.matrix }} | ||||||||||||
| steps: | ||||||||||||
| - name: Checkout code | ||||||||||||
| uses: actions/checkout@v4 | ||||||||||||
| with: | ||||||||||||
| fetch-depth: 0 | ||||||||||||
|
|
||||||||||||
| - name: Detect changes to build inputs | ||||||||||||
| id: detect | ||||||||||||
| run: | | ||||||||||||
| # 只在影响镜像构建的文件变更时才触发构建,避免无关改动白白排队。 | ||||||||||||
| # workflow_dispatch 始终构建。 | ||||||||||||
| if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then | ||||||||||||
| echo "has-changes=true" >> "$GITHUB_OUTPUT" | ||||||||||||
| exit 0 | ||||||||||||
| fi | ||||||||||||
| BEFORE="${{ github.event.before }}" | ||||||||||||
| AFTER="${{ github.event.after }}" | ||||||||||||
| if [ -z "$BEFORE" ] || [ "$BEFORE" = "0000000000000000000000000000000000000000" ]; then | ||||||||||||
| echo "has-changes=true" >> "$GITHUB_OUTPUT" | ||||||||||||
| exit 0 | ||||||||||||
| fi | ||||||||||||
| CHANGED=$(git diff --name-only "$BEFORE" "$AFTER" || true) | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🟡 Nit — Do not suppress
Proposed fix- CHANGED=$(git diff --name-only "$BEFORE" "$AFTER" || true)
+ if ! CHANGED=$(git diff --name-only "$BEFORE" "$AFTER"); then
+ echo "::error::Cannot determine changed build inputs"
+ exit 1
+ fiAs per coding guidelines, run the silent-failure-hunter agent to detect swallowed errors, inappropriate fallbacks, and missing error propagation. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||
| PATTERNS='^(docker/Dockerfile|\.cargo/config\.toml|bindings/python/pyproject\.toml|model_gateway/|crates/|Cargo\.lock)' | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🔴 Important — Include the workflow file in build-input detection. A change to Proposed fix- PATTERNS='^(docker/Dockerfile|\.cargo/config\.toml|bindings/python/pyproject\.toml|model_gateway/|crates/|Cargo\.lock)'
+ PATTERNS='^(\.github/workflows/build-docker-images\.yml|docker/Dockerfile|\.cargo/config\.toml|bindings/python/pyproject\.toml|model_gateway/|crates/|Cargo\.lock)'📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
| if printf '%s\n' "$CHANGED" | grep -qE "$PATTERNS"; then | ||||||||||||
| echo "has-changes=true" >> "$GITHUB_OUTPUT" | ||||||||||||
| else | ||||||||||||
| echo "has-changes=false" >> "$GITHUB_OUTPUT" | ||||||||||||
| fi | ||||||||||||
|
|
||||||||||||
| - name: Build matrix | ||||||||||||
| id: matrix | ||||||||||||
| run: | | ||||||||||||
| # 单镜像仓库,matrix 只有一行;保留 include 结构便于后续多 app 扩展。 | ||||||||||||
| MATRIX=$(cat <<'JSON' | ||||||||||||
| {"include":[{"app":"smg","dockerfile":"docker/Dockerfile","context":"."}]} | ||||||||||||
| JSON | ||||||||||||
| ) | ||||||||||||
| # jq -c 压成单行 JSON 给 fromJSON 用 | ||||||||||||
| echo "matrix=$(printf '%s' "$MATRIX" | jq -c .)" >> "$GITHUB_OUTPUT" | ||||||||||||
|
|
||||||||||||
| # ------------------------------------------------------------------ | ||||||||||||
| # 阶段 2: 构建 (Build Job) | ||||||||||||
| # ------------------------------------------------------------------ | ||||||||||||
| build-and-push: | ||||||||||||
| name: Build ${{ matrix.app }} | ||||||||||||
| needs: configure | ||||||||||||
| if: needs.configure.outputs.has-changes == 'true' | ||||||||||||
| runs-on: astro-web3-arc-runner-set | ||||||||||||
| permissions: | ||||||||||||
| contents: read | ||||||||||||
| packages: write | ||||||||||||
| strategy: | ||||||||||||
| fail-fast: false | ||||||||||||
| matrix: | ||||||||||||
| include: ${{ fromJSON(needs.configure.outputs.matrix) }} | ||||||||||||
| steps: | ||||||||||||
| - name: Export pod env to GitHub env | ||||||||||||
| run: | | ||||||||||||
| echo "ZOT_REGISTRY=${ZOT_REGISTRY}" >> $GITHUB_ENV | ||||||||||||
| echo "CLUSTER_GOPROXY=${CLUSTER_GOPROXY}" >> $GITHUB_ENV | ||||||||||||
| echo "BUILDKIT_HOST=${BUILDKIT_HOST}" >> $GITHUB_ENV | ||||||||||||
|
Comment on lines
+82
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- workflow files ---'
git ls-files '.github/workflows/build-docker-images.yml'
printf '%s\n' '--- workflow structure ---'
wc -l .github/workflows/build-docker-images.yml
sed -n '1,230p' .github/workflows/build-docker-images.yml
printf '%s\n' '--- relevant references ---'
rg -n -C 3 'ZOT_REGISTRY|BUILDKIT_HOST|CLUSTER_GOPROXY|GITHUB_ENV|GITHUB_OUTPUT|git diff|change|paths|permissions|persist-credentials|configure' \
.github/workflows/build-docker-images.ymlRepository: smg-project/smg Length of output: 13284 🌐 Web query:
💡 Result: In GitHub Actions, GITHUB_ENV is used to persist environment variables across steps within the same job [1][2]. When you write a key-value pair to the GITHUB_ENV file (e.g., echo "MY_VAR=value" >> $GITHUB_ENV), that variable becomes available to all subsequent steps in that job [1][3]. Key considerations include: 1. Scope: The step that writes to GITHUB_ENV does not have access to the new variable value immediately [1][2]. It only becomes accessible in subsequent steps [1]. 2. Context Accessibility: Once a variable is set via GITHUB_ENV, you can access it in two primary ways in later steps: - Shell Environment Variables: Use standard shell syntax (e.g., $MY_VAR for Bash or Citations:
🌐 Web query:
💡 Result: To use an environment variable set via GITHUB_ENV in a conditional (if statement) within a GitHub Actions workflow, you must reference it using the env context [1]. Variables appended to GITHUB_ENV in one step become available as environment variables in all subsequent steps within the same job [1]. Important distinctions: 1. GITHUB_ENV vs. env context: Variables set via GITHUB_ENV are available as standard shell environment variables in subsequent steps [1]. However, because if conditionals are evaluated by GitHub Actions before the job is sent to the runner, you must access these values using the env context (e.g., ${{ env.VAR_NAME }}) rather than standard shell syntax (e.g., $VAR_NAME) [1]. 2. Availability: Variables set in GITHUB_ENV are scoped to the specific job where they were defined and are not automatically passed to other jobs [1]. Example workflow: name: GITHUB_ENV Conditional Example on: workflow_dispatch jobs: example-job: runs-on: ubuntu-latest steps: - name: Set environment variable run: echo "MY_CONDITION=true" >> Citations: 🌐 Web query:
💡 Result: In GitHub Actions, GITHUB_ENV is a mechanism that allows workflows to set environment variables that persist for subsequent steps [1]. It operates via the FileCommandManager in the runner's architecture [2]. How it works: 1. File Commands: At the beginning of each step, the runner creates a unique temporary file path designated for environment variable updates, which is exposed to the step via the GITHUB_ENV environment variable [1]. 2. Processing: When a step writes a key-value pair to this file (e.g., echo "MY_VAR=value" >> Citations:
🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
workflow = Path(".github/workflows/build-docker-images.yml").read_text()
for name in ("ZOT_REGISTRY", "BUILDKIT_HOST", "CLUSTER_GOPROXY"):
export_line = re.search(rf'echo "{name}=\$\{{{name}\}}" >> "\$GITHUB_ENV"', workflow)
env_refs = re.findall(rf'\$\{{\{{\s*env\.{name}\s*\}}\}}', workflow)
print(f"{name}: exported_to_GITHUB_ENV={bool(export_line)}, later_env_context_refs={len(env_refs)}")
for name in ("ZOT_REGISTRY", "BUILDKIT_HOST"):
print(f"{name}_required_check_present=", bool(re.search(rf'\$\{{{name}:\?', workflow)))
print("BUILDKIT_HOST_consumers:")
for line_no, line in enumerate(workflow.splitlines(), 1):
if "BUILDKIT_HOST" in line:
print(f"{line_no}: {line}")
print("ZOT_REGISTRY_consumers:")
for line_no, line in enumerate(workflow.splitlines(), 1):
if "ZOT_REGISTRY" in line:
print(f"{line_no}: {line}")
PY
printf '%s\n' '--- GitHub runner env-context ADR excerpts ---'
curl -fsSL https://raw.githubusercontent.com/actions/runner/main/docs/adrs/0278-env-context.md \
| rg -n -C 3 'GITHUB_ENV|environment file|env context|subsequent|step'Repository: smg-project/smg Length of output: 2533 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- current runner implementation ---'
curl -fsSL https://raw.githubusercontent.com/actions/runner/main/src/Runner.Worker/FileCommandManager.cs \
| rg -n -C 8 'SetEnvFileCommand|SetEnvContext|Global.EnvironmentVariables|GITHUB_ENV'
printf '%s\n' '--- corrected workflow probe ---'
python3 - <<'PY'
from pathlib import Path
import re
lines = Path(".github/workflows/build-docker-images.yml").read_text().splitlines()
for name in ("ZOT_REGISTRY", "BUILDKIT_HOST", "CLUSTER_GOPROXY"):
exported = any(
re.search(rf'\b{name}=\$\{{{name}\}}\s*>>\s*\$GITHUB_ENV\b', line)
for line in lines
)
refs = sum(f"${{{{ env.{name} }}}}" in line for line in lines)
checks = any(f'"${{{name}:?' in line for line in lines)
print(f"{name}: GITHUB_ENV_export={exported}, env_context_refs={refs}, required_check={checks}")
PYRepository: smg-project/smg Length of output: 3589 Fail fast when required runner configuration is missing. 🧰 Tools🪛 actionlint (1.7.12)[error] 83-83: shellcheck reported issue in this script: SC2086:info:1:40: Double quote to prevent globbing and word splitting (shellcheck) [error] 83-83: shellcheck reported issue in this script: SC2086:info:2:46: Double quote to prevent globbing and word splitting (shellcheck) [error] 83-83: shellcheck reported issue in this script: SC2086:info:3:42: Double quote to prevent globbing and word splitting (shellcheck) [error] 83-83: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects (shellcheck) 🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||
|
|
||||||||||||
| - name: Checkout code | ||||||||||||
| uses: actions/checkout@v4 | ||||||||||||
|
|
||||||||||||
| - name: Set up Docker Buildx | ||||||||||||
| id: setup-buildx | ||||||||||||
| uses: docker/setup-buildx-action@v3 | ||||||||||||
| with: | ||||||||||||
| driver: remote | ||||||||||||
| endpoint: ${{ env.BUILDKIT_HOST }} | ||||||||||||
|
|
||||||||||||
| - name: Log in to GitHub Container Registry | ||||||||||||
| uses: docker/login-action@v3 | ||||||||||||
| with: | ||||||||||||
| registry: ${{ env.REGISTRY }} | ||||||||||||
| username: ${{ github.actor }} | ||||||||||||
| password: ${{ secrets.GITHUB_TOKEN }} | ||||||||||||
|
|
||||||||||||
| # 阿里云镜像仓库偶发 TLS 握手超时(自建 runner 到杭州个人版实例网络抖动), | ||||||||||||
| # docker/login-action 内部只 login 一次无重试,这里改成带退避的重试。 | ||||||||||||
| - name: Log in to Aliyun Registry | ||||||||||||
| env: | ||||||||||||
| ALIYUN_REGISTRY: ${{ env.ALIYUN_REGISTRY }} | ||||||||||||
| ALIYUN_USERNAME: ${{ secrets.ALIYUN_USERNAME }} | ||||||||||||
| ALIYUN_PASSWORD: ${{ secrets.ALIYUN_PASSWORD }} | ||||||||||||
| run: | | ||||||||||||
| max_attempts=5 | ||||||||||||
| for attempt in $(seq 1 "$max_attempts"); do | ||||||||||||
| if printf '%s' "$ALIYUN_PASSWORD" | docker login "$ALIYUN_REGISTRY" -u "$ALIYUN_USERNAME" --password-stdin; then | ||||||||||||
| echo "Aliyun login succeeded on attempt $attempt" | ||||||||||||
| exit 0 | ||||||||||||
| fi | ||||||||||||
| echo "Aliyun login attempt $attempt failed, backing off before retry..." | ||||||||||||
| sleep $((attempt * 5)) | ||||||||||||
| done | ||||||||||||
| echo "::error::Aliyun registry login failed after $max_attempts attempts" | ||||||||||||
| exit 1 | ||||||||||||
|
|
||||||||||||
| - name: Metadata | ||||||||||||
| id: meta | ||||||||||||
| uses: docker/metadata-action@v5 | ||||||||||||
| with: | ||||||||||||
| images: | | ||||||||||||
| ${{ env.REGISTRY }}/${{ env.IMAGE_PREFIX }}-${{ matrix.app }} | ||||||||||||
| ${{ env.ZOT_REGISTRY }}/${{ env.IMAGE_PREFIX }}-${{ matrix.app }} | ||||||||||||
| ${{ env.ALIYUN_REGISTRY }}/${{ env.ALIYUN_NAMESPACE }}/1xtoken-${{ matrix.app }} | ||||||||||||
| tags: | | ||||||||||||
| type=sha,prefix=${{ github.ref_name }}-,suffix=-${{ github.run_number }} | ||||||||||||
|
|
||||||||||||
| # 构建与推送拆分:构建是确定性故障(pnpm install、Dockerfile 语法、编译错误), | ||||||||||||
| # 重试无意义,失败直接在这一步标红并阻断 job,报错落在真实出错位置; | ||||||||||||
| # 推送才可能因阿里云个人版仓库鉴权/网络抖动失败,值得退避重试。 | ||||||||||||
| - name: Build image | ||||||||||||
| id: build | ||||||||||||
| uses: docker/build-push-action@v6 | ||||||||||||
| with: | ||||||||||||
| context: ${{ matrix.context || '.' }} | ||||||||||||
| file: ${{ matrix.dockerfile }} | ||||||||||||
| push: false | ||||||||||||
| load: false | ||||||||||||
| cache-from: type=registry,ref=${{ env.ZOT_REGISTRY }}/cache/${{ env.IMAGE_PREFIX }}-${{ matrix.app }}:buildcache | ||||||||||||
| cache-to: type=registry,ref=${{ env.ZOT_REGISTRY }}/cache/${{ env.IMAGE_PREFIX }}-${{ matrix.app }}:buildcache,mode=min | ||||||||||||
|
|
||||||||||||
| # 阿里云鉴权端点偶发 connection reset by peer(push 时拉 oauth token 网络抖动), | ||||||||||||
| # docker/build-push-action 内部不重试推送,这里用「首尝试 + 2 次退避重试 + 状态校验」兜底。 | ||||||||||||
| # 构建产物已在 BuildKit 缓存里,重试几乎只重推 manifest,成本可忽略。 | ||||||||||||
| - name: Push image | ||||||||||||
| id: push-1 | ||||||||||||
| continue-on-error: true | ||||||||||||
| uses: docker/build-push-action@v6 | ||||||||||||
| with: | ||||||||||||
| context: ${{ matrix.context || '.' }} | ||||||||||||
| file: ${{ matrix.dockerfile }} | ||||||||||||
| push: true | ||||||||||||
| tags: ${{ steps.meta.outputs.tags }} | ||||||||||||
| cache-from: type=registry,ref=${{ env.ZOT_REGISTRY }}/cache/${{ env.IMAGE_PREFIX }}-${{ matrix.app }}:buildcache | ||||||||||||
|
|
||||||||||||
| - name: Retry push (attempt 2) | ||||||||||||
| if: steps.push-1.outcome == 'failure' | ||||||||||||
| run: | | ||||||||||||
| echo "::warning::首次推送失败(可能阿里云鉴权端点网络抖动),退避 5s 后重试..." | ||||||||||||
| sleep 5 | ||||||||||||
|
|
||||||||||||
| - name: Push image (retry 2) | ||||||||||||
| id: push-2 | ||||||||||||
| continue-on-error: true | ||||||||||||
| if: steps.push-1.outcome == 'failure' | ||||||||||||
| uses: docker/build-push-action@v6 | ||||||||||||
| with: | ||||||||||||
| context: ${{ matrix.context || '.' }} | ||||||||||||
| file: ${{ matrix.dockerfile }} | ||||||||||||
| push: true | ||||||||||||
| tags: ${{ steps.meta.outputs.tags }} | ||||||||||||
| cache-from: type=registry,ref=${{ env.ZOT_REGISTRY }}/cache/${{ env.IMAGE_PREFIX }}-${{ matrix.app }}:buildcache | ||||||||||||
|
|
||||||||||||
| - name: Retry push (attempt 3) | ||||||||||||
| if: steps.push-2.outcome == 'failure' | ||||||||||||
| run: | | ||||||||||||
| echo "::warning::第二次推送失败,退避 10s 后最后一次重试..." | ||||||||||||
| sleep 10 | ||||||||||||
|
|
||||||||||||
| - name: Push image (retry 3) | ||||||||||||
| id: push-3 | ||||||||||||
| continue-on-error: true | ||||||||||||
| if: steps.push-2.outcome == 'failure' | ||||||||||||
| uses: docker/build-push-action@v6 | ||||||||||||
| with: | ||||||||||||
| context: ${{ matrix.context || '.' }} | ||||||||||||
| file: ${{ matrix.dockerfile }} | ||||||||||||
| push: true | ||||||||||||
| tags: ${{ steps.meta.outputs.tags }} | ||||||||||||
| cache-from: type=registry,ref=${{ env.ZOT_REGISTRY }}/cache/${{ env.IMAGE_PREFIX }}-${{ matrix.app }}:buildcache | ||||||||||||
|
|
||||||||||||
| # 三次推送任一成功即通过;全败则标红并阻断 job。 | ||||||||||||
| - name: Verify push result | ||||||||||||
| if: always() | ||||||||||||
| run: | | ||||||||||||
| if [ "${{ steps.push-1.outcome }}" = "success" ] \ | ||||||||||||
| || [ "${{ steps.push-2.outcome }}" = "success" ] \ | ||||||||||||
| || [ "${{ steps.push-3.outcome }}" = "success" ]; then | ||||||||||||
| echo "镜像推送成功" | ||||||||||||
| else | ||||||||||||
| echo "::error::镜像推送在 3 次尝试后均失败(阿里云个人版仓库鉴权/网络问题)" | ||||||||||||
| exit 1 | ||||||||||||
| fi | ||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -644,6 +644,45 @@ pub enum PolicyConfig { | |
| cache_boundaries: Vec<usize>, | ||
| }, | ||
|
|
||
| /// Cache-aware length policy: cache affinity with a long/short pool split | ||
| /// driven by the `pool` worker label (`pool=long` → long pool, otherwise | ||
| /// short pool). Step 1-3 mirror `cache_aware` (string tree only); step 4 | ||
| /// routes by uncached prefill tokens. See `policies/cache_aware_length.rs`. | ||
| #[serde(rename = "cache_aware_length")] | ||
| CacheAwareLength { | ||
| /// Minimum matched-prefix share before a request pins to a holder. | ||
| #[serde(alias = "cache_match_threshold")] | ||
| #[serde(default = "default_cal_cache_threshold")] | ||
| cache_threshold: f32, | ||
| /// Spill gate, absolute part: the global imbalance fires when the | ||
| /// healthy-fleet load spread exceeds this. | ||
| #[serde(alias = "spill_abs_threshold")] | ||
| #[serde(default = "default_cal_balance_abs_threshold")] | ||
| balance_abs_threshold: usize, | ||
| /// Spill gate, relative part (multiple of the healthy-fleet min load); | ||
| /// fires only together with `balance_abs_threshold`. | ||
| #[serde(alias = "spill_rel_threshold")] | ||
| #[serde(default = "default_cal_balance_rel_threshold")] | ||
| balance_rel_threshold: f32, | ||
| #[serde(default = "default_cal_eviction_interval_secs")] | ||
| eviction_interval_secs: u64, | ||
| #[serde(default = "default_cal_max_tree_size")] | ||
| max_tree_size: usize, | ||
| /// Divisor for char-level token estimation when `X-Prompt-Tokens` is | ||
| /// absent (default 4). | ||
| #[serde(default = "default_cal_chars_per_token")] | ||
| chars_per_token: usize, | ||
| /// Uncached-prefill-token boundary between long and short requests. | ||
| #[serde(default = "default_cal_long_prefill_threshold")] | ||
| long_prefill_threshold: usize, | ||
| /// Load ceiling for the long pool (`pool=long` workers). | ||
| #[serde(default = "default_cal_long_pool_max_load")] | ||
| long_pool_max_load: usize, | ||
| /// Load ceiling for the short pool (remaining workers). | ||
| #[serde(default = "default_cal_short_pool_max_load")] | ||
| short_pool_max_load: usize, | ||
| }, | ||
|
Comment on lines
+647
to
+684
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find policy-name enumerations and policy config surfaces in the bindings and Go SDK.
set -euo pipefail
fd -t f -e py -e go -e rs . bindings 2>/dev/null | head -50
rg -n --glob '!target/**' -C4 '"cache_aware"' bindings || echo "no cache_aware literal under bindings/"
rg -n --glob '!target/**' -C4 'cache_aware_length' . || echo "no cache_aware_length outside model_gateway/"
rg -n --glob '!target/**' -C4 'prefix_hash|least_load' --type=go . || echo "no Go policy enumeration found"Repository: smg-project/smg Length of output: 50371 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- Python policy surfaces ---'
sed -n '1,120p' bindings/python/src/smg/router_args.py
sed -n '1,90p' bindings/python/src/smg/router.py
rg -n -C3 'policy_from_str|PolicyType|COMMON_POLICY_CHOICES|cache_aware_length|policy' bindings/python/src/smg bindings/python/src/lib.rs
echo '--- Go SDK policy surfaces ---'
sed -n '1,130p' bindings/golang/multi_client.go
sed -n '320,365p' bindings/golang/src/policy.rs
rg -n -C3 'PolicyName|policy_name|cache_aware_length|cache_aware' bindings/golang --glob '*.go' --glob '*.rs' --glob '!internal/proto/**'
echo '--- Rust policy enum and Python conversion ---'
rg -n -C5 'enum PolicyType|CacheAware|policy_from_str|PolicyConfig' bindings/python model_gateway/src --glob '*.rs' --glob '*.py' | head -240Repository: smg-project/smg Length of output: 50371 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
args = Path("bindings/python/src/smg/router_args.py").read_text()
router = Path("bindings/python/src/smg/router.py").read_text()
lib = Path("bindings/python/src/lib.rs").read_text()
go = Path("bindings/golang/src/policy.rs").read_text()
checks = {
"Python CLI common choices": '"cache_aware_length"' in args[args.index("COMMON_POLICY_CHOICES"):args.index("PREFILL_POLICY_CHOICES")],
"Python string conversion": '"cache_aware_length"' in router[router.index("policy_map = {"):router.index("return policy_map")],
"Python PolicyType variant": re.search(r"\bCacheAwareLength\b", lib) is not None,
"Python Rust conversion arm": re.search(r"PolicyType::CacheAwareLength\s*=>", lib) is not None,
"Go SDK cache_aware_length parser": '"cache_aware_length"' in go,
}
for name, value in checks.items():
print(f"{name}: {value}")
print("\nPython PolicyType declaration:")
start = lib.index("pub enum PolicyType")
print(lib[start:lib.index("}", start) + 1])
print("\nGo supported-policy error text:")
for line in go.splitlines():
if "Supported policies:" in line:
print(line.strip())
PYRepository: smg-project/smg Length of output: 721 🔴 Important Add The Go SDK uses a separate client-side policy path. Its omission does not block 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| /// Power-of-two choices policy: samples two workers and routes to the one | ||
| /// with the lower expected wait, scored like `least_load` | ||
| /// (`(queued_tokens + inflight_tokens) / throughput + kv_pressure_weight * k/(1-k)`). | ||
|
|
@@ -777,6 +816,35 @@ fn default_cache_ttl_secs() -> u64 { | |
| 180 | ||
| } | ||
|
|
||
| // cache_aware_length defaults (kept aligned with CacheAwareLengthConfig::default). | ||
| fn default_cal_cache_threshold() -> f32 { | ||
| 0.3 | ||
| } | ||
| fn default_cal_balance_abs_threshold() -> usize { | ||
| 32 | ||
| } | ||
| fn default_cal_balance_rel_threshold() -> f32 { | ||
| 1.1 | ||
| } | ||
| fn default_cal_eviction_interval_secs() -> u64 { | ||
| 30 | ||
| } | ||
| fn default_cal_max_tree_size() -> usize { | ||
| 10000 | ||
| } | ||
| fn default_cal_chars_per_token() -> usize { | ||
| 4 | ||
| } | ||
| fn default_cal_long_prefill_threshold() -> usize { | ||
| 100_000 | ||
| } | ||
| fn default_cal_long_pool_max_load() -> usize { | ||
| 4 | ||
| } | ||
| fn default_cal_short_pool_max_load() -> usize { | ||
| 32 | ||
| } | ||
|
|
||
| fn default_prefix_token_count() -> usize { | ||
| 256 | ||
| } | ||
|
|
@@ -828,6 +896,7 @@ impl PolicyConfig { | |
| PolicyConfig::RoundRobin => "round_robin", | ||
| PolicyConfig::Passthrough => "passthrough", | ||
| PolicyConfig::CacheAware { .. } => "cache_aware", | ||
| PolicyConfig::CacheAwareLength { .. } => "cache_aware_length", | ||
| PolicyConfig::PowerOfTwo { .. } => "power_of_two", | ||
| PolicyConfig::LeastLoad { .. } => "least_load", | ||
| PolicyConfig::Bucket { .. } => "bucket", | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -565,6 +565,81 @@ impl ConfigValidator { | |||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| PolicyConfig::CacheAwareLength { | ||||||||||||||||||||||||||||||
| cache_threshold, | ||||||||||||||||||||||||||||||
| balance_abs_threshold: _, | ||||||||||||||||||||||||||||||
| balance_rel_threshold, | ||||||||||||||||||||||||||||||
| eviction_interval_secs, | ||||||||||||||||||||||||||||||
| max_tree_size, | ||||||||||||||||||||||||||||||
| chars_per_token, | ||||||||||||||||||||||||||||||
| long_prefill_threshold, | ||||||||||||||||||||||||||||||
| long_pool_max_load, | ||||||||||||||||||||||||||||||
| short_pool_max_load, | ||||||||||||||||||||||||||||||
| } => { | ||||||||||||||||||||||||||||||
| if !(0.0..=1.0).contains(cache_threshold) { | ||||||||||||||||||||||||||||||
| return Err(ConfigError::InvalidValue { | ||||||||||||||||||||||||||||||
| field: "cache_threshold".to_string(), | ||||||||||||||||||||||||||||||
| value: cache_threshold.to_string(), | ||||||||||||||||||||||||||||||
| reason: "Must be between 0.0 and 1.0".to_string(), | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if *balance_rel_threshold < 1.0 { | ||||||||||||||||||||||||||||||
| return Err(ConfigError::InvalidValue { | ||||||||||||||||||||||||||||||
| field: "balance_rel_threshold".to_string(), | ||||||||||||||||||||||||||||||
| value: balance_rel_threshold.to_string(), | ||||||||||||||||||||||||||||||
| reason: "Must be >= 1.0".to_string(), | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+587
to
+593
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔴 Important Reject a non-finite
🛡️ Proposed fix- if *balance_rel_threshold < 1.0 {
+ if !balance_rel_threshold.is_finite() || *balance_rel_threshold < 1.0 {
return Err(ConfigError::InvalidValue {
field: "balance_rel_threshold".to_string(),
value: balance_rel_threshold.to_string(),
- reason: "Must be >= 1.0".to_string(),
+ reason: "Must be finite and >= 1.0".to_string(),
});
}As per coding guidelines: "Do not silently fall back to None or a default when configuration validation should fail loudly." 📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if *eviction_interval_secs == 0 { | ||||||||||||||||||||||||||||||
| return Err(ConfigError::InvalidValue { | ||||||||||||||||||||||||||||||
| field: "eviction_interval_secs".to_string(), | ||||||||||||||||||||||||||||||
| value: eviction_interval_secs.to_string(), | ||||||||||||||||||||||||||||||
| reason: "Must be > 0".to_string(), | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if *max_tree_size == 0 { | ||||||||||||||||||||||||||||||
| return Err(ConfigError::InvalidValue { | ||||||||||||||||||||||||||||||
| field: "max_tree_size".to_string(), | ||||||||||||||||||||||||||||||
| value: max_tree_size.to_string(), | ||||||||||||||||||||||||||||||
| reason: "Must be > 0".to_string(), | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if *chars_per_token == 0 { | ||||||||||||||||||||||||||||||
| return Err(ConfigError::InvalidValue { | ||||||||||||||||||||||||||||||
| field: "chars_per_token".to_string(), | ||||||||||||||||||||||||||||||
| value: chars_per_token.to_string(), | ||||||||||||||||||||||||||||||
| reason: "Must be > 0".to_string(), | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if *long_prefill_threshold == 0 { | ||||||||||||||||||||||||||||||
| return Err(ConfigError::InvalidValue { | ||||||||||||||||||||||||||||||
| field: "long_prefill_threshold".to_string(), | ||||||||||||||||||||||||||||||
| value: long_prefill_threshold.to_string(), | ||||||||||||||||||||||||||||||
| reason: "Must be > 0".to_string(), | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if *long_pool_max_load == 0 { | ||||||||||||||||||||||||||||||
| return Err(ConfigError::InvalidValue { | ||||||||||||||||||||||||||||||
| field: "long_pool_max_load".to_string(), | ||||||||||||||||||||||||||||||
| value: long_pool_max_load.to_string(), | ||||||||||||||||||||||||||||||
| reason: "Must be > 0".to_string(), | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if *short_pool_max_load == 0 { | ||||||||||||||||||||||||||||||
| return Err(ConfigError::InvalidValue { | ||||||||||||||||||||||||||||||
| field: "short_pool_max_load".to_string(), | ||||||||||||||||||||||||||||||
| value: short_pool_max_load.to_string(), | ||||||||||||||||||||||||||||||
| reason: "Must be > 0".to_string(), | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| PolicyConfig::PowerOfTwo { | ||||||||||||||||||||||||||||||
| load_check_interval_secs, | ||||||||||||||||||||||||||||||
| } => { | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: smg-project/smg
Length of output: 50372
🌐 Web query:
GitHub Actions workflow permissions unspecified job default GITHUB_TOKEN permissions actions checkout persist-credentials true documentation💡 Result:
When GitHub Actions workflow permissions are not explicitly defined in a workflow or job, the GITHUB_TOKEN inherits the default settings configured at the enterprise, organization, or repository level [1][2]. Depending on your repository's settings, this default may be either "read and write" for all scopes or "read" access only for specific scopes like contents and packages [3]. It is a recommended security practice to explicitly define the permissions needed for each job using the permissions key to follow the principle of least privilege [4][5]. Regarding the
actions/checkoutaction, thepersist-credentialsinput determines whether the authentication token (either the GITHUB_TOKEN or a provided SSH key) is persisted in the local Git configuration [6][7]. The default value forpersist-credentialsistrue, which allows your scripts to run authenticated Git commands during the job [6][8][9]. The token is removed during the post-job cleanup [6][7]. Note that settingpersist-credentialstofalseprevents the action from configuring these credentials, requiring you to manage authentication for Git operations manually if needed [6][7].Citations:
Restrict token scope and disable credential persistence.
Add
permissions: contents: readtoconfigure. Setpersist-credentials: falseon bothactions/checkout@v4steps.🧰 Tools
🪛 actionlint (1.7.12)
[error] 22-22: label "astro-web3-arc-runner-set" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2025-vs2026", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xlarge", "macos-latest-large", "macos-26-intel", "macos-26-xlarge", "macos-26-large", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xlarge", "macos-14-large", "macos-14", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🪛 zizmor (1.29.0)
[warning] 27-30: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 20-68: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 Prompt for AI Agents
Source: Linters/SAST tools