Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 78 additions & 36 deletions .github/workflows/qwen-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4547,6 +4547,34 @@ jobs:
[ .[] | select((.user.login // "") == $ab)
| select((.body // "") | contains("<!-- autofix-base-updated"))
| .created_at ] | max // ""' "${WORKDIR}/ic.json")"
# The instant THIS round's net was measured. Stamped into the
# growth-now marker so the divergence read filters on measurement
# time, not the marker comment's created_at — the report posts the
Comment on lines +4551 to +4552

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Recorded rather than changed, per your second option: the sibling GROWTH_BASELINE reader now carries a KNOWN RESIDUAL comment naming the skew, its narrow window (the base update must land inside the anchor round's own agent run) and the first-wins property that keeps it from being re-poisoned later. Stamping measured= into the growth-base marker is tracked with the rest of #9114 rather than widening this change again.

# marker only after the agent's ~120-minute run, so a round in
# flight when a concurrent base update lands would otherwise pass a
# created_at filter while carrying pre-update sums (#9114 R2-6).
MEASURED_AT="$(date -u +%Y-%m-%dT%H:%M:%SZ)"
echo "measured_at=${MEASURED_AT}" >> "${GITHUB_OUTPUT}"
Comment on lines +4556 to +4557

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[Suggestion] R4-5: The comparability filter compares values from two domains: measured= is a runner wall-clock stamp (here) while the cutoff BASE_UPD_AT is a GitHub-server created_at — and the admit-direction residual needs NO clock skew at all. Inside prepare, the ic.json fetch (~4317) precedes this stamp by seconds of local computation, so a base update (autofix-base-updated, created_at = B) landing inside that span admits this round's fully pre-update sums past the cutoff; the stamp also postdates the checkout-fixed tree (~4289), so the checkout→stamp span is exposed the same way. A poisoned sum persists for the rest of the window, and nothing logs which sums were incomparable. This is strictly narrower than pre-change behavior (a residual of the #9114 R2-6 fix, not a regression), and Risk & Scope names the clock source but not the admit-direction consequence; the sibling residual (growth-base read) carries a KNOWN RESIDUAL comment while this one does not.

— Failure scenario: a concurrent scheduled-scan base update lands between this round's ic.json fetch and its stamp. The round's marker carries measured= > B while its sums were measured against the pre-update tree; the next round's cutoff filter admits it, and PREV_SUM/OVER_ROUNDS_PRIOR are computed against an incomparable sum — a false escalation, or a still-climbing diff read as shrinking (suppressed handoff). Runner-clock-ahead skew widens the window; the runner-behind drop arm is unreachable on a disciplined clock (verified and rejected).

Witness: not run — the claim requires a concurrent scheduled-scan base update landing inside a live address job's prepare, which only GitHub Actions can produce; verdict rests on the line-level trace (fetch 4317 → stamp 4556 ordering in one bash block, string-comparison semantics) plus probe-verified filter behavior.

Suggested fix — document beside GROWTH_NOW_CUTOFF, mirroring the sibling KNOWN RESIDUAL block (4563–4572): the cross-domain comparison, the [ic.json fetch, stamp] admit window, and the drop-arm unreachability; and/or track it in #9114. (A clock-free causal alternative, unverified: after measuring, re-check for a base-update marker absent from the snapshot the sums came from, and treat the round as unmeasured if one appeared.)

中文说明

[Suggestion] R4-5:可比性过滤在比较两个不同来源的值:measured= 是 runner 墙钟打点(此处),而 cutoff BASE_UPD_AT 是 GitHub 服务器的 created_at —— 而且「放入」方向的残留问题不需要任何时钟偏差就能成立。prepare 内部,ic.json 抓取(约 4317 行)比该打点早数秒的本地计算,因此一次 base 更新(autofix-base-updated,created_at = B)若落在这个区间内,本轮按旧树测得的 sum 会被 cutoff 放入可比窗口;该打点同样晚于 checkout 固定的树(约 4289 行),checkout→打点区间同样暴露。被污染的 sum 会在窗口余下轮次持续存在,且没有任何日志能区分哪些 sum 不可比。这严格窄于改动前的行为(是 #9114 R2-6 修复的残留,而非回归),Risk & Scope 提到了时钟来源但没有提到「放入」方向的后果;兄弟残留(growth-base 读取)有 KNOWN RESIDUAL 注释,而这里没有。

— 失败场景:一次并发的 scheduled-scan base 更新落在本轮 ic.json 抓取与打点之间。本轮 marker 携带 measured= > B,但其 sum 是按更新前的树测得;下一轮 cutoff 过滤将其放入,PREV_SUM/OVER_ROUNDS_PRIOR 基于不可比的 sum 计算 —— 误升级,或仍在增长的 diff 被读作收缩(接管被抑制)。runner 时钟偏快会加宽窗口;runner 时钟偏慢的丢弃分支在受控时钟下不可达(已验证并否决)。

证据:未运行 —— 该主张需要一次并发的 scheduled-scan base 更新恰好落在运行中 address job 的 prepare 内,只有 GitHub Actions 能产生;裁决基于行级追踪(fetch 4317 → 打点 4556 在同一 bash 块中的顺序、字符串比较语义)加探测验证过的过滤行为。

建议修复 —— 在 GROWTH_NOW_CUTOFF 旁仿照兄弟 KNOWN RESIDUAL 块(4563–4572)补充文档:跨时钟域比较、[ic.json 抓取, 打点] 放入窗口、丢弃分支不可达;并/或记录到 #9114。(一个无时钟的因果替代方案,未验证:测量完成后复查是否出现了快照中不存在的 base-update marker,若出现则视本轮为未测量。)

— qwen3.8-max via Qwen Code /review (v0.21.11)

# NOTE (#9114 R2-8/R6-3): re-anchoring on an EXTERNAL head move (an
# author push) is deliberately NOT done here. The obvious signal —
# comparing the checked-out head against the bot's last judged head
# (autofix-redcheck) — is wrong: that marker records the head the
# agent was GIVEN, before its own push, so it differs after every
# pushing round and would re-anchor on the bot's own fixes, zeroing
# the census in exactly the push regime the handoff exists for. A
# correct version needs both a bot-authored-move test and a
# PERSISTED cut (a one-round cut is re-admitted the next round);
# that is its own change, tracked in #9114.
Comment on lines +4568 to +4570

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agreed — the claims went stale when b328bd3 dropped the head-move re-anchor, and the merged record must not say R2-8 is fixed. This checkout cannot apply the fix: the PR title/body/test plan and the #9114 tracking comment are GitHub-side artifacts, and the address-review agent has no GitHub write access (the workflow owns those mutations; title/body files are only consumed on the initial PR-creation path). Suggested reconciliation for a maintainer (or a workflow pass with write access):

Leaving this thread open until the metadata is reconciled.

中文说明

同意——b328bd37f2 移除 head 移动重锚之后,这些声明已过时,合并记录不应写 R2-8 已修复。本检出无法执行该修复:PR 标题/正文/测试计划与 #9114 跟踪评论都是 GitHub 侧产物,address-review agent 没有 GitHub 写权限(这些写操作由 workflow 持有;标题/正文文件只在首次建 PR 的路径上被消费)。建议维护者(或有写权限的 workflow 步骤)按以下方式对齐:

在元数据对齐之前保持该线程开放。

GROWTH_NOW_CUTOFF="${BASE_UPD_AT}"
# KNOWN RESIDUAL (#9114): this sibling read still filters on the
# comment's created_at, not a prepare-time measured= like the
# growth-now read below. Its values are measured in prepare too, so a
# round whose agent run straddles a base update can anchor the window
# on pre-update values. Narrow (the update must land inside the
# anchor round's own agent run) and first-wins, so it cannot be
# re-poisoned later in the window; stamping measured= into the
# growth-base marker is tracked with the rest of #9114 rather than
# widening this change.
GROWTH_BASELINE="$(jq -r --arg ab "${AUTOFIX_BOT}" --arg key "${LIVE_REARM_KEY}" --arg baseupd "${BASE_UPD_AT}" '
[ .[] | select((.user.login // "") == $ab) | . as $c | ($c.body // "")
| [ scan("<!-- autofix-growth-base src=(-?[0-9]+) test=(-?[0-9]+) key=([^ ]+) -->") ] | .[]
Expand Down Expand Up @@ -4605,7 +4633,7 @@ jobs:
# climbing anyway. Read this window's prior per-round growth markers
# (written by the report step): count the rounds that were over
# budget, and take the MOST RECENT prior over-budget run's growth
# SUM (highest run_id — see below; NOT the window-wide max, which a
# SUM (latest measured= — see below; NOT the window-wide max, which a
# one-off spike would raise forever). The round is DIVERGING when it
# is over budget now, the brake has already fired for
# >= GROWTH_DIVERGENCE_ROUNDS prior rounds, and the diff has NOT
Expand All @@ -4620,39 +4648,46 @@ jobs:
# Count runs whenever the net is measured (not only over budget), so
# the trajectory clause below is accurate even on a round that pulled
# back under budget. markers:
# <!-- autofix-growth-now src=N test=N over=BOOL round=N run=ID key=W -->
# Deduped and ordered by run=GITHUB_RUN_ID, the per-workflow-run id:
# the report post's bounded retry re-posts one run's marker (same
# run_id → collapses to the LATEST by created_at, so a re-run's
# fresher attempt wins over its own stale first post), while every
# distinct address run has a fresh, monotonically increasing run_id. round=/eval-watermark are NOT a
# safe identity — a state-triggered lane (a persistent merge conflict
# selects the PR every scan with no new evaluable feedback) freezes
# both NEWEST and ROUND, so distinct over-budget runs would share them
# and collapse, stalling the count. Filtered to markers AFTER any
# mid-window base update (BASE_UPD_AT) — the same re-anchoring the
# growth-base read guards against. The "not shrinking" test compares
# against the MOST RECENT prior over-budget run's sum (highest
# run_id), not the window-wide max: a single transient spike would
# otherwise raise the bar forever and a genuine plateau-over-budget
# runaway (the exact case to escalate) would never clear it. The
# CURRENT run's own markers are excluded (run != GITHUB_RUN_ID): a
# re-run of a failed job keeps the same run id and its failed
# attempt already posted a marker, so counting it would over-report
# the round's own attempt as a PRIOR one.
# <!-- autofix-growth-now src=N test=N over=BOOL round=N run=ID measured=TS key=W -->
# Deduped by run=GITHUB_RUN_ID (the per-workflow-run id) and ORDERED
# by measured=: the report post's bounded retry re-posts one run's
# marker, and a failed job's re-run keeps the same run_id, so a run
# collapses to its LATEST measurement — and that collapse happens
# BEFORE the over/window/cutoff filters, or a re-run that came back
# under budget would still be represented by its stale over=true
# attempt. Every distinct address run has a fresh run_id.
# round=/eval-watermark are NOT a safe identity — a state-triggered
# lane (a persistent merge conflict selects the PR every scan with no
# new evaluable feedback) freezes both NEWEST and ROUND, so distinct
# over-budget runs would share them and collapse, stalling the count.
# Filtered on measured= (the prepare-time measurement instant, NOT
# the comment's post-agent created_at) after GROWTH_NOW_CUTOFF, so a
# prior sum measured against a pre-base-update tree is dropped rather
# than compared to this round's. measured= is OPTIONAL in the scan:
# markers posted before it existed fall back to their comment's
# created_at, so deploying this does not blank the census of a window
# that is already in flight.
# The "not shrinking" test compares against the MOST RECENT prior
# over-budget run's sum (latest measured=), not the window-wide max: a
# single transient spike would otherwise raise the bar forever and a
# genuine plateau-over-budget runaway (the exact case to escalate)
# would never clear it. The CURRENT run's own markers are excluded
# (run != GITHUB_RUN_ID): a re-run of a failed job keeps the same run
# id and its failed attempt already posted a marker, so counting it
# would over-report the round's own attempt as a PRIOR one.
GROWTH_DIVERGED='false'
OVER_ROUNDS_PRIOR=0
PREV_SUM=0
if [[ "${NET_MEASURED}" == 'true' ]]; then
read -r OVER_ROUNDS_PRIOR PREV_SUM < <(jq -r --arg ab "${AUTOFIX_BOT}" --arg key "${LIVE_REARM_KEY}" --arg baseupd "${BASE_UPD_AT}" --arg curr "${GITHUB_RUN_ID}" '
read -r OVER_ROUNDS_PRIOR PREV_SUM < <(jq -r --arg ab "${AUTOFIX_BOT}" --arg key "${LIVE_REARM_KEY}" --arg cutoff "${GROWTH_NOW_CUTOFF}" --arg curr "${GITHUB_RUN_ID}" '
[ .[] | select((.user.login // "") == $ab) | . as $c | ($c.body // "")
| [ scan("<!-- autofix-growth-now src=(-?[0-9]+) test=(-?[0-9]+) over=([a-z]+) round=([0-9]+) run=([0-9]+) key=([^ ]+) -->") ] | .[]
| {sum: ((.[0] | tonumber) + (.[1] | tonumber)), over: .[2], round: (.[3] | tonumber), run: (.[4] | tonumber), win: .[5], at: ($c.created_at // "")} ]
| [ scan("<!-- autofix-growth-now src=(-?[0-9]+) test=(-?[0-9]+) over=([a-z]+) round=([0-9]+) run=([0-9]+) (?:measured=([^ ]+) )?key=([^ ]+) -->") ] | .[]
| {sum: ((.[0] | tonumber) + (.[1] | tonumber)), over: .[2], round: (.[3] | tonumber), run: (.[4] | tonumber), measured: (.[5] // ($c.created_at // "")), win: .[6]} ]
| group_by(.run) | map(max_by(.measured))

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed and closed. The interaction you describe is real — it comes from combining the R1-9 fix (collapse before the over filter) with the R1-4 fix (created_at fallback): an inert over=false marker with no measured= sorts ~2h later than the same run's real prepare-time measurement and wins the collapse, after which the over filter drops the run entirely.

The code fix landed in cbb7186 (max_by([.explicit, .measured]), with explicit: (.[5] != null)), but it was unpinned — I mutation-checked it: reverting to max_by(.measured) left the whole suite green. bf290a1 adds the regression fixture for exactly your scenario (run 1001: attempt 1 over=true measured=M1, attempt 2 inert over=false with no measured= and a later created_at) → false 1; the mutant flips it to false 0 and now fails.

| map(select(.win == $key and .over == "true"))
| map(select(.run != ($curr | tonumber)))
| map(select($baseupd == "" or (.at > $baseupd)))
| group_by(.run) | map(max_by(.at))
| sort_by(.run)
| map(select($cutoff == "" or (.measured > $cutoff)))
| sort_by(.measured)
| "\(length) \((last.sum) // 0)"' "${WORKDIR}/ic.json" 2> /dev/null || echo "0 0")
[[ "${OVER_ROUNDS_PRIOR}" =~ ^[0-9]+$ ]] || OVER_ROUNDS_PRIOR=0
[[ "${PREV_SUM}" =~ ^-?[0-9]+$ ]] || PREV_SUM=0
Expand Down Expand Up @@ -5476,6 +5511,7 @@ jobs:
GROWTH_SRC: '${{ steps.prepare.outputs.growth_src }}'
GROWTH_TEST: '${{ steps.prepare.outputs.growth_test }}'
CRITICAL_ONLY_GROWTH: '${{ steps.prepare.outputs.critical_only_growth }}'
MEASURED_AT: '${{ steps.prepare.outputs.measured_at }}'
run: |-
# gh has its own $GITHUB_ENV-injectable channels: pin the host and
# drop any planted token BEFORE the identity check below, so a
Expand Down Expand Up @@ -5864,9 +5900,10 @@ jobs:
echo "<!-- autofix-growth-base src=${GROWTH_BASE_SRC} test=${GROWTH_BASE_TEST} key=${GROWTH_BASE_WIN:-${WINDOW:-none}} -->"
fi
# Per-round growth history the next round's divergence read
# counts; run=GITHUB_RUN_ID is the dedup+order identity (a retry
# re-posts the same run, distinct runs get fresh run ids).
echo "<!-- autofix-growth-now src=${GROWTH_SRC:-0} test=${GROWTH_TEST:-0} over=${CRITICAL_ONLY_GROWTH:-false} round=${NEXT_ROUND} run=${GITHUB_RUN_ID} key=${GROWTH_BASE_WIN:-${WINDOW:-none}} -->"
# counts; run=GITHUB_RUN_ID is the DEDUP identity (a retry or a
# job re-run re-posts the same run; measured= orders and picks
# that run's latest attempt).
echo "<!-- autofix-growth-now src=${GROWTH_SRC:-0} test=${GROWTH_TEST:-0} over=${CRITICAL_ONLY_GROWTH:-false} round=${NEXT_ROUND} run=${GITHUB_RUN_ID} measured=${MEASURED_AT} key=${GROWTH_BASE_WIN:-${WINDOW:-none}} -->"
} > "${WORKDIR}/report.md"
STATUS="pushed (round ${NEXT_ROUND}/${MAX_ROUNDS})"
else
Expand Down Expand Up @@ -5896,9 +5933,10 @@ jobs:
echo "<!-- autofix-growth-base src=${GROWTH_BASE_SRC} test=${GROWTH_BASE_TEST} key=${GROWTH_BASE_WIN:-${WINDOW:-none}} -->"
fi
# Per-round growth history the next round's divergence read
# counts; run=GITHUB_RUN_ID is the dedup+order identity (a retry
# re-posts the same run, distinct runs get fresh run ids).
echo "<!-- autofix-growth-now src=${GROWTH_SRC:-0} test=${GROWTH_TEST:-0} over=${CRITICAL_ONLY_GROWTH:-false} round=${ROUND} run=${GITHUB_RUN_ID} key=${GROWTH_BASE_WIN:-${WINDOW:-none}} -->"
# counts; run=GITHUB_RUN_ID is the DEDUP identity (a retry or a
# job re-run re-posts the same run; measured= orders and picks
# that run's latest attempt).
echo "<!-- autofix-growth-now src=${GROWTH_SRC:-0} test=${GROWTH_TEST:-0} over=${CRITICAL_ONLY_GROWTH:-false} round=${ROUND} run=${GITHUB_RUN_ID} measured=${MEASURED_AT} key=${GROWTH_BASE_WIN:-${WINDOW:-none}} -->"
} > "${WORKDIR}/report.md"
STATUS="no action needed"
fi
Expand Down Expand Up @@ -6050,10 +6088,14 @@ jobs:
# over-budget round that never reaches 'Push and report' leaves a
# history gap and the divergence count under-reports. Empty outputs
# (prepare never ran) fall through the :-0/:-false marker fallbacks
# to an inert over=false entry.
# to an inert over=false entry — measured= then OMITS itself (an
# EMPTY measured= value matches no scan and would silently drop the
# marker these fallbacks exist to keep); the reader falls back to
# the comment's created_at.
GROWTH_SRC: '${{ steps.prepare.outputs.growth_src }}'
GROWTH_TEST: '${{ steps.prepare.outputs.growth_test }}'
CRITICAL_ONLY_GROWTH: '${{ steps.prepare.outputs.critical_only_growth }}'
MEASURED_AT: '${{ steps.prepare.outputs.measured_at }}'
GROWTH_BASE_WIN: '${{ steps.prepare.outputs.growth_base_win }}'
run: |-
# The head the agent actually evaluated — captured in prepare before
Expand Down Expand Up @@ -6563,8 +6605,8 @@ jobs:
# Per-round growth history the divergence read counts — same
# marker the push/no-op report paths write, so an over-budget
# round that timed out or was gate-rejected is not a gap. run=
# (per-workflow-run) is the dedup/order identity.
echo "<!-- autofix-growth-now src=${GROWTH_SRC:-0} test=${GROWTH_TEST:-0} over=${CRITICAL_ONLY_GROWTH:-false} round=${MARK_ROUND} run=${GITHUB_RUN_ID} key=${GROWTH_BASE_WIN:-${WINDOW:-none}} -->"
# (per-workflow-run) is the DEDUP identity; measured= orders.
echo "<!-- autofix-growth-now src=${GROWTH_SRC:-0} test=${GROWTH_TEST:-0} over=${CRITICAL_ONLY_GROWTH:-false} round=${MARK_ROUND} run=${GITHUB_RUN_ID}${MEASURED_AT:+ measured=${MEASURED_AT}} key=${GROWTH_BASE_WIN:-${WINDOW:-none}} -->"
# A sentinel ts means the agent evaluated NOTHING (crash, API
# error, gate crash) and the next scan must retry. Recording a
# judged head here would make RED_HEAD == LIVE_HEAD, so the
Expand Down
Loading
Loading