mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
fix(ci): interleave PR review endpoint cohorts and report final routes (#16151)
* feat(ci): support same-model endpoint splits in PR review canary The endpoint override derived the cohort by comparing the routed model name against the configured canary/default models, so configuring the SAME model on both cohorts — a pure quota split across two gateways — labelled every PR 'default' and silently disabled the override. Derive the cohort from the bucket predicate itself instead (review_cohort_for_bucket: same clamp, same PR-number bucket test as the model router), so model name equality no longer matters. The notice's cohort field remains the attribution when both cohorts run one model. * fix(ci): interleave review endpoint buckets and report final routes Co-authored-by: Qwen-Coder <[email protected]> * style(ci): apply Prettier to the review endpoint bucket test The Lint & Static lane runs `node scripts/lint.js --prettier` on the files a PR touches, and this PR's own test file was not formatted: three `expect(...).toBe(...)` lines exceed the print width, so the incremental lint step exited 1 on every head. Formatting only - no assertion or fixture changed. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmvzj4h0j9a * fix(ci): share the endpoint host derivation and pin the cohort notice - Extract openai_base_host() so the proxy bypass and the cohort notice derive the host from one place; the notice cannot name a host the traffic was not routed through. - Drop the comments that still describe the removed model router. - Pin the notice's bucket count (8% -> bucket 87/100), make the noKey/badUrl rows exact strings, and assert initial_openai_host so the three mutations that survived this suite now fail. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmvznesli9g --------- Co-authored-by: Qwen-Coder <[email protected]> Co-authored-by: yiliang114 <[email protected]>
This commit is contained in:
4 files changed
+260
-102
No files matched your search
@@ -1050,7 +1050,7 @@ jobs:
|
||||
# of reviews to a second model. All three arrive as variables so a
|
||||
# rollout is a repository-variable change, never a workflow edit:
|
||||
# an empty canary or percent keeps today's single model, and
|
||||
# review_model_for_bucket below turns them into OPENAI_MODEL.
|
||||
# the bucket selection below turns them into OPENAI_MODEL.
|
||||
REVIEW_MODEL_DEFAULT: '${{ vars.QWEN_PR_REVIEW_MODEL }}'
|
||||
REVIEW_MODEL_CANARY: '${{ vars.QWEN_PR_REVIEW_MODEL_CANARY }}'
|
||||
REVIEW_MODEL_CANARY_PERCENT: '${{ vars.QWEN_PR_REVIEW_MODEL_CANARY_PERCENT }}'
|
||||
@@ -1199,13 +1199,20 @@ jobs:
|
||||
fail "qwen CLI is required on the review runner."
|
||||
fi
|
||||
|
||||
# shellcheck disable=SC2016
|
||||
# Shared by the proxy bypass below and the cohort notice, so the
|
||||
# notice cannot name a host the traffic was not routed through.
|
||||
openai_base_host() {
|
||||
node -e 'console.log(new URL(process.env.OPENAI_BASE_URL).hostname)'
|
||||
}
|
||||
|
||||
# shellcheck disable=SC2016
|
||||
configure_qwen_network() {
|
||||
local openai_host proxy_bin
|
||||
if ! command -v node >/dev/null 2>&1; then
|
||||
fail "node is required to parse OPENAI_BASE_URL for the proxy bypass."
|
||||
fi
|
||||
openai_host="$(node -e 'console.log(new URL(process.env.OPENAI_BASE_URL).hostname)')"
|
||||
openai_host="$(openai_base_host)"
|
||||
if [ -z "$openai_host" ]; then
|
||||
fail "Could not parse a hostname from OPENAI_BASE_URL."
|
||||
fi
|
||||
@@ -1440,7 +1447,7 @@ jobs:
|
||||
|
||||
echo "qwen_path=$(command -v qwen)"
|
||||
qwen --version
|
||||
echo "openai_host=${openai_host}"
|
||||
echo "initial_openai_host=${openai_host}"
|
||||
echo "qwen_http_proxy=disabled"
|
||||
if [ -n "${QWEN_CI_HTTPS_PROXY}${QWEN_CI_https_proxy}${QWEN_CI_HTTP_PROXY}${QWEN_CI_http_proxy}" ]; then
|
||||
echo "child_git_github_proxy=restored"
|
||||
@@ -1671,9 +1678,9 @@ jobs:
|
||||
PROMPT="${PROMPT} --comment"
|
||||
fi
|
||||
|
||||
# The percent both consumers read — the router's bucket test and the
|
||||
# cohort notice below — normalized once so the notice cannot report a
|
||||
# split the router did not apply. Shape follows SALVAGE_ELAPSED_PERCENT
|
||||
# The percent both consumers read — the bucket test and the cohort
|
||||
# notice below — normalized once so the notice cannot report a split
|
||||
# the bucket test did not apply. Shape follows SALVAGE_ELAPSED_PERCENT
|
||||
# (strip leading zeros, bound the digit count) but clamps to the
|
||||
# opposite pole: there 100 means cede, here it would mean full
|
||||
# rollout, so a typo on a repository variable must land on off. Keep
|
||||
@@ -1696,34 +1703,6 @@ jobs:
|
||||
echo "$percent"
|
||||
}
|
||||
|
||||
# Canary routing for the review model. A PR's NUMBER picks its bucket
|
||||
# for the PR's whole life, so every re-review of the same PR — and
|
||||
# every finding one round is judged against the next — stays on one
|
||||
# model; a per-run bucket would flip models mid-PR and make two rounds
|
||||
# of reviews incomparable. Percent zero means off, and an unresolvable
|
||||
# PR number keeps the default: the canary must never be the reason a
|
||||
# review breaks.
|
||||
# KEEP THE SHAPE: scripts/tests/qwen-pr-review-workflow.test.js slices
|
||||
# both functions plus the call site and executes the real bash, so the
|
||||
# helper stays above the router and both braces sit at column 0.
|
||||
review_model_for_bucket() {
|
||||
local default_model="$1" canary_model="$2" pr="$4"
|
||||
local percent
|
||||
percent="$(canary_percent_effective "$3")"
|
||||
case "$pr" in
|
||||
''|*[!0-9]*)
|
||||
echo "$default_model"
|
||||
return 0
|
||||
;;
|
||||
esac
|
||||
if [ -n "$canary_model" ] && [ "$percent" -gt 0 ] &&
|
||||
[ $((10#$pr % 100)) -lt "$percent" ]; then
|
||||
echo "$canary_model"
|
||||
else
|
||||
echo "$default_model"
|
||||
fi
|
||||
}
|
||||
|
||||
# A padded canary name (" qwen3-coder-plus ") would reach --model
|
||||
# verbatim, the provider would reject it, and that failure is
|
||||
# terminal for the whole cohort on every re-review round — the same
|
||||
@@ -1736,10 +1715,26 @@ jobs:
|
||||
|
||||
CANARY_PERCENT_EFFECTIVE="$(canary_percent_effective \
|
||||
"${REVIEW_MODEL_CANARY_PERCENT:-}")"
|
||||
OPENAI_MODEL="$(review_model_for_bucket \
|
||||
"${REVIEW_MODEL_DEFAULT:-}" "$REVIEW_MODEL_CANARY" \
|
||||
"$CANARY_PERCENT_EFFECTIVE" "${PR_NUMBER:-}")"
|
||||
export OPENAI_MODEL
|
||||
REVIEW_BUCKET_COUNT=100
|
||||
if [ "$(( CANARY_PERCENT_EFFECTIVE % 10 ))" -eq 0 ]; then
|
||||
REVIEW_BUCKET_COUNT=10
|
||||
fi
|
||||
REVIEW_BUCKET=unknown
|
||||
REVIEW_COHORT=default
|
||||
case "${PR_NUMBER:-}" in
|
||||
''|*[!0-9]*) ;;
|
||||
*)
|
||||
REVIEW_BUCKET="$(( 10#$PR_NUMBER % REVIEW_BUCKET_COUNT ))"
|
||||
if [ -n "$REVIEW_MODEL_CANARY" ] &&
|
||||
[ "$(( REVIEW_BUCKET * 100 ))" -lt "$(( CANARY_PERCENT_EFFECTIVE * REVIEW_BUCKET_COUNT ))" ]; then
|
||||
REVIEW_COHORT=canary
|
||||
fi
|
||||
;;
|
||||
esac
|
||||
OPENAI_MODEL="${REVIEW_MODEL_DEFAULT:-}"
|
||||
if [ "$REVIEW_COHORT" = canary ]; then
|
||||
OPENAI_MODEL="$REVIEW_MODEL_CANARY"
|
||||
fi
|
||||
|
||||
# The canary cohort may carry its own endpoint as well as its own
|
||||
# model: REVIEW_BASE_URL_CANARY/REVIEW_API_KEY_CANARY move the
|
||||
@@ -1752,32 +1747,26 @@ jobs:
|
||||
REVIEW_BASE_URL_CANARY="${REVIEW_BASE_URL_CANARY:-}"
|
||||
REVIEW_BASE_URL_CANARY="${REVIEW_BASE_URL_CANARY#"${REVIEW_BASE_URL_CANARY%%[![:space:]]*}"}"
|
||||
REVIEW_BASE_URL_CANARY="${REVIEW_BASE_URL_CANARY%"${REVIEW_BASE_URL_CANARY##*[![:space:]]}"}"
|
||||
REVIEW_COHORT=default
|
||||
if [ -n "$REVIEW_MODEL_CANARY" ] &&
|
||||
[ "${OPENAI_MODEL:-}" = "$REVIEW_MODEL_CANARY" ] &&
|
||||
[ "$OPENAI_MODEL" != "${REVIEW_MODEL_DEFAULT:-}" ]; then
|
||||
REVIEW_COHORT=canary
|
||||
fi
|
||||
if [ "$REVIEW_COHORT" = canary ] && [ -n "$REVIEW_BASE_URL_CANARY" ]; then
|
||||
if [ -n "${REVIEW_API_KEY_CANARY:-}" ] &&
|
||||
node -e 'try{if(!new URL(process.env.REVIEW_BASE_URL_CANARY).host)process.exit(1)}catch{process.exit(1)}' 2>/dev/null; then
|
||||
OPENAI_BASE_URL="$REVIEW_BASE_URL_CANARY"
|
||||
OPENAI_API_KEY="$REVIEW_API_KEY_CANARY"
|
||||
export OPENAI_BASE_URL OPENAI_API_KEY
|
||||
else
|
||||
echo "::warning::canary endpoint unusable (REVIEW_OPENAI_API_KEY_CANARY missing or QWEN_PR_REVIEW_BASE_URL_CANARY unparseable); falling back to the default cohort."
|
||||
OPENAI_MODEL="${REVIEW_MODEL_DEFAULT:-}"
|
||||
REVIEW_COHORT=default
|
||||
export OPENAI_MODEL
|
||||
fi
|
||||
fi
|
||||
export OPENAI_MODEL OPENAI_BASE_URL OPENAI_API_KEY
|
||||
REVIEW_ENDPOINT_HOST="$(openai_base_host)"
|
||||
# The run's own record of which cohort served this review: the model
|
||||
# is not carried anywhere else a later comparison could read it (the
|
||||
# cost ledger has tokens, not a model), so a rollout that cannot be
|
||||
# told apart afterwards is unmeasurable. Both fields are the resolved
|
||||
# ones — the raw percent would certify a cohort the router clamped
|
||||
# away.
|
||||
echo "::notice title=Review model::${OPENAI_MODEL:-<CLI default>} (PR #${PR_NUMBER:-?}, canary ${CANARY_PERCENT_EFFECTIVE}%, cohort ${REVIEW_COHORT})"
|
||||
# told apart afterwards is unmeasurable. Every field is the resolved
|
||||
# one — the raw percent would certify a split the bucket test
|
||||
# clamped away.
|
||||
echo "::notice title=Review model::${OPENAI_MODEL:-<CLI default>} (PR #${PR_NUMBER:-?}, canary ${CANARY_PERCENT_EFFECTIVE}%, bucket ${REVIEW_BUCKET}/${REVIEW_BUCKET_COUNT}, cohort ${REVIEW_COHORT}, endpoint ${REVIEW_ENDPOINT_HOST})"
|
||||
|
||||
MODEL_ARGS=()
|
||||
if [ -n "${OPENAI_MODEL:-}" ]; then
|
||||
|
||||
Reference in new issue
Block a user