Skip to content

docs(agentx): correct the warmup-grace CHANGELOG to what shipped - #1379

Open
zengleixin-amd wants to merge 1 commit into
mainfrom
feature/leixin/agentx-grace-doc-fix
Open

docs(agentx): correct the warmup-grace CHANGELOG to what shipped#1379
zengleixin-amd wants to merge 1 commit into
mainfrom
feature/leixin/agentx-grace-doc-fix

Conversation

@zengleixin-amd

Copy link
Copy Markdown
Contributor

Summary

Two sentences in the #1309 CHANGELOG entry describe behaviour that later commits in that same PR removed. The entry was written against the first version of the change and never re-synced, so it now documents a safety property that does not exist.

Documentation only — no code change.

What is wrong

1. The anchor does not default to 8; the scaling is opt-in.

The entry says:

AGENTX_WARMUP_GRACE_CONC declares the concurrency the grace was measured at and defaults to 8, so every existing configuration derives exactly what it derived before.

baseline.py::agentx_warmup_grace_sec (main):

if not _agentx_positive_int(src, "AGENTX_WARMUP_GRACE_CONC"):
    return grace

No anchor → no scaling at all. The stated conclusion happens to still hold, but for the opposite reason. The problem is the mechanism an operator infers from it: read as "CONC scaling is automatic", a grace measured at conc=8 is expected to scale at CONC=32 and does not. That branch also logs nothing, so the round is silently under-budgeted.

Concretely, using this entry's own Kimi-K3 measurements (conc=8 → ~3000s, conc=16 → ~5000s): a 3600s grace at CONC=32 needs roughly 10000s and gets 3600s. The round does not fail — it reports a prefix-reuse figure taken before the cache filled, which is the exact failure #1309 set out to fix.

2. Sweep variants do not re-derive their grace.

The entry says:

A sweep variant re-derives from its own CONC after the variant envs are merged, since the switch runs before that concurrency exists.

That re-derivation was reverted. _grid_runner.py now carries the opposite instruction and the reason:

# NOT re-derived per variant. ...
# Doing that here is WRONG while the two caps above it are session-scaled.
# ...the round is SIGKILLed at 10800s while warmup is still draining --
# strictly worse than leaving the grace alone, which is what shipped before.

Why bother with a CHANGELOG note

Because it is the only human-facing documentation this knob has:

CHANGELOG.md                             <- the only description
docs/                                    <- no AGENTX_* variable appears at all
aiperf_client.sh header                  <- correctly omits it (consumed orchestrator-side)

There is nothing to fall back on, and the wrong direction is the dangerous one — it promises protection that is not there, in the one area where being wrong yields plausible-but-invalid numbers rather than a crash.

Documenting the whole AGENTX_* family in docs/reference/environment-variables.md is a real gap, but a separate and much larger job; adding one knob to a reference that documents none of its siblings would be worse than leaving it. This PR only stops the one description that exists from being wrong.

Scope

  • The rewritten paragraph states the opt-in rule, why assuming the anchor was rejected, and what happens when you run above the concurrency you measured at.
  • A new Not covered clause records that sweep variants stay session-scaled, and why.

Test plan

  • No code changed — git diff --stat is CHANGELOG.md | 38 +++----
  • Both claims verified against origin/main source, not against the branch they were written on

Two sentences in the #1309 entry describe behaviour that later commits in that
same PR removed. The entry was written against the first version and never
re-synced, so it now documents a safety property that does not exist.

  - "AGENTX_WARMUP_GRACE_CONC ... defaults to 8, so every existing
    configuration derives exactly what it derived before." The anchor is
    opt-in: with none declared, agentx_warmup_grace_sec returns the grace flat
    and never scales. The conclusion happens to still hold, but for the
    opposite reason, and an operator who reads it as "CONC scaling is
    automatic" gets an under-budgeted warmup at every concurrency above the
    one they measured at -- silently, since that branch logs nothing.

  - "A sweep variant re-derives from its own CONC after the variant envs are
    merged." That re-derivation was reverted. _grid_runner.py now carries the
    opposite instruction ("NOT re-derived per variant") and explains why doing
    it while the two caps above it stay session-scaled is strictly worse than
    leaving the grace alone.

This matters more than a stale note usually would because the CHANGELOG is the
only human-facing documentation of AGENTX_WARMUP_GRACE_CONC -- the AGENTX_*
family does not appear in docs/ at all, and the client header does not carry
this knob because it is consumed orchestrator-side. Documenting the family
properly is a separate, larger job; this only stops the one description that
exists from being wrong.

The failure it points at is the one #1309 set out to fix: a warmup cut short
does not fail the round, it reports a prefix-reuse figure taken before the
cache filled.

No code change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zengleixin-amd
zengleixin-amd requested a review from a team as a code owner September 2, 2026 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant