Skip to content

fix(agentx): refuse a baseline measured with the prefix cache off - #1884

Merged
chaojhou merged 9 commits into
mainfrom
fix/agentx-keep-radix-cache
Oct 16, 2026
Merged

chaojhou merged 9 commits into
mainfrom
fix/agentx-keep-radix-cache

Conversation

@zoroyihan7

@zoroyihan7 zoroyihan7 commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator
  • Description: what and why

    An AgentX trace replay is mostly shared prefix (the 062126 corpus reports a theoretical prefix hit near 97% with inputs averaging ~143K tokens). When the server for an AgentX baseline runs with its prefix cache off, the baseline measures a cold cache on every turn (on SGLang: #cached-token: 0 on every prefill batch, TTFT p50 ~68 s), and any candidate that turns the cache back on is graded against that baseline and credited with the cache itself as a gain (measured: +2341% on e2e_norm_intvty_p50, +210% output throughput). Today Magpie's generic SGLang launchers append --disable-radix-cache by default, ATOM keeps its cache off unless --enable_prefix_caching is passed (its seed grid ships that flag as the atom_prefix_cache candidate), and an operator can pass --disable-radix-cache / --no-enable-prefix-caching through --server-args.

    This PR only adds a guard on the baseline; it does not change how the server is launched. Keeping the cache on at launch is left to feat(agentx) Use Magpie native AgentX for Hyperloom optimization #1675 (Magpie's native AgentX path).

    The guard (refuse_cacheless_agentx_baseline, run by BaselineExecutor._run after the measurement, only for a genuine baseline, i.e. the same _should_establish_quality_ref test that gates the quality reference, and only when agentx_active(shared_state)): a succeeded baseline is marked failed with error_class=agentx_baseline_prefix_cache_off when the server ran with the cache off by its engine's own account in launch_evidence:

    • SGLang: the resolved observed_server_config has disable_radix_cache=True;
    • vLLM: its non-default launch record shows enable_prefix_caching=False;
    • ATOM: it prints no launch record and keeps the cache off by default, so the requested server args (EXTRA_ATOM_ARGS from the materialized config, which Magpie's ATOM launcher passes as is) lack --enable_prefix_caching.

    A launch whose account cannot be read (no record, or one past the parser's cap) is left alone. The executor only marks the result; loop/writeback.py decides what the run does with it: is_promotable_result does not promote it (a baseline is otherwise judged by its measurement fields alone, and the refused round keeps its throughput, so it would have become the anchor), the class is in _STOPS_ON_FIRST_OCCURRENCE (beside ARGV_INVALID), so the run stops with stop_reason=baseline_failed on the first occurrence, including a genuine re-baseline after an anchor exists (_arms_baseline_failure_gates), and _authorable_launch_log excludes it, so the refusal never records a boot trigger or a launch observation for enablement (the server booted and measured fine).

    Gating on AgentX rather than on a client-reported prefix hit covers both AgentX clients (aiperf and MLPerf, which reports no hit). A baseline with no launch evidence or of another framework, a synthetic baseline, and every candidate (explore, profile, sweep, and kernel-validation candidates that run as baseline-kind tasks with quality_ref_exempt) are left alone.

    Choices: refuse rather than flag, because a flagged baseline still becomes the reference every later KEEP is graded against. Stop instead of retrying, because the next attempt would launch the same server.

  • Linked issue(s): none. The launch-side fix is feat(agentx) Use Magpie native AgentX for Hyperloom optimization #1675.

  • Tests: src/hyperloom/inference_optimizer/tests/test_agentx_prefix_cache.py covers each engine's decision (SGLang resolved setting, including through the real ServerArgs parser, and an unread setting left alone; vLLM through the real launch-record parser, including the absent key meaning on; ATOM with and without the flag in either spelling, and through build_launch_evidence from a materialized config), an unrecorded launch left alone, the guard through BaselineExecutor._run (refused for each engine and the stop left to writeback, AgentX enabled by state or by environment, synthetic baseline not judged, kept when the cache was reused, an already-failed baseline keeps its own cause, candidates and quality_ref_exempt baseline-kind tasks not judged), and one writeback-level test with enablement on, for the first baseline and for a re-baseline after an anchor: a refused round with real throughput is not promotable while a cache-on one is, and through handle_unpromotable_result the run stops on the first refusal, no enablement trigger is recorded, no launch log is stashed, the failure streak is untouched (fails before the writeback change). Mutation check: dropping the class from is_promotable_result, from _authorable_launch_log's exclusion, or from _arms_baseline_failure_gates, or the AgentX gate in BaselineExecutor._run, each fails a test. Ran the new file (26 passed), every test file that references is_promotable_result, handle_unpromotable_result, _authorable_launch_log, ARGV_INVALID, baseline_eval_failed, baseline_tput or the guard (5219 passed; the three failures, test_trace_analyze_handler_dry_run_returns_structured_result, test_enablement_zero_patch_round_does_not_erase_prior_accepted_work and test_credentials_validate_and_reset_claude_config, fail the same way on the previous head in my environment), every test file that references BaselineExecutor, _workload_envs, agentx_active or launch_log_evidence (all pass except test_trace_analyze_handler_dry_run_returns_structured_result, which fails the same way on the previous head in my environment), and ruff check / ruff format --check.

  • Size/complexity triggers crossed: module length of baseline.py, which is already over its recorded baseline on main (4292 lines against 4253 recorded). This change adds 4 lines there (an import and one guarded call in BaselineExecutor._run, where every genuine baseline measurement passes); the guard itself lives in a new module, _agentx_prefix_cache.py. Module length of loop/writeback.py, likewise already over its recorded baseline (4792 lines at the merge base against 4748 recorded): this change adds 23 lines there (the _STOPS_ON_FIRST_OCCURRENCE table with its reasons, the exclusion in _authorable_launch_log, the is_promotable_result rule and the _arms_baseline_failure_gates predicate), while handle_unpromotable_result, itself in the baseline, gets 2 lines shorter with unchanged complexity. Labelled baseline-raise for those two module rows; shrinking either module is unrelated to this fix.

  • If this simplifies or refactors: handle_unpromotable_result's ARGV_INVALID branch becomes a lookup in _STOPS_ON_FIRST_OCCURRENCE, which the new class joins; ARGV_INVALID behaves as before and the function's complexity is unchanged (C901 25, cognitive 55).

  • Observable effect: an AgentX baseline measured with the prefix cache off now fails with agentx_baseline_prefix_cache_off, naming the engine and the evidence, and the run stops as baseline_failed without opening enablement, instead of becoming the reference for inflated gains. Until feat(agentx) Use Magpie native AgentX for Hyperloom optimization #1675 lands, an AgentX SGLang run whose launcher still disables the cache stops at the baseline with that error; an AgentX ATOM run needs --server-args "--enable_prefix_caching". Documented in SKILL.md.

  • Breaking changes: no interface change. An AgentX run whose baseline server has the prefix cache off now stops at the baseline.

  • PR addresses single concern: yes (the baseline must have measured with the prefix cache on).

  • Root cause is upstream (Magpie/TraceLens/GEAK/IntelliKit/AgentKernelArena), ticket filed: the launcher default is addressed by feat(agentx) Use Magpie native AgentX for Hyperloom optimization #1675.

🤖 Generated with Claude Code

@zoroyihan7
zoroyihan7 requested a review from haofrank as a code owner October 14, 2026 10:44
@zoroyihan7 zoroyihan7 added the domain:inference Related to inference optimization label Oct 14, 2026
@zoroyihan7
zoroyihan7 requested a review from a team as a code owner October 14, 2026 10:44
@zoroyihan7 zoroyihan7 added the type:bug Something is broken or not working as expected label Oct 14, 2026
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Code metrics gate: PASSED with waived findings (baseline-raise)

Judged: the 6 files this change touches. Files it does not touch never fail it; anything found there is listed under Outside this change for information.

Dimension Limit New Worse Baseline out of date Baseline entries vs base
Cyclomatic complexity fails > 20 0 0 0 72 +0
Cognitive complexity fails > 30 0 0 0 284 +0
Function length (lines) fails > 80 0 0 0 449 +0
Nested block depth fails > 5 0 0 0 27 +0
Module length (lines) warns > 800, fails > 1200 0 2 0 53 +0
Duplicated lines per file fails > 0 0 0 0 118 +0
Dead code (vulture) fails > 0 0 0 0 2 +0
Check Findings Effect
Comments 0
English only 0
Production code imports test code 0
Repeated literals 0
A1 Private name of another package 0
A8 Flag argument 0
A9 Tuple return 0
A10 Raw vocabulary comparison 0
A11 Hidden global write 0
A12 Pure forwarder 0
A12 Single-implementation abstraction 0

Worse than the baseline (waived by the baseline-raise label): 2

Unit Dimension Baseline Now
src/hyperloom/orchestrator/actions/executors/baseline.py:1 Module length (lines) 4253 4296
src/hyperloom/orchestrator/loop/writeback.py:1 Module length (lines) 4748 4830

How to fix

  • New or worse: split the unit (extract a function, a class, a module) until it is back under the limit, or no worse than its recorded value. The baseline cannot absorb a new violation: CI refuses any baseline entry added or raised relative to the base branch.
  • Baseline out of date: something improved. Run python scripts/code_metrics.py --update-baseline and commit scripts/code_metrics_baseline.txt; the command only lowers or removes entries.
  • Override: the baseline-raise PR label waives new, worse and baseline-growth findings and the failing design checks (they stay listed here). Use it only with the reason in the PR description; it waives nothing else.
  • Tools for a local run: pip install ruff==0.16.2 complexipy==8.0.1 vulture==2.16 and npm install -g jscpd@5.4.1 (or point CODE_METRICS_JSCPD at another jscpd command, such as npx --yes jscpd@<version>).

Already over at the base (backlog, not this change's): 5

Unit Dimension Base Now
src/hyperloom/orchestrator/loop/writeback.py:3528 WritebackCollaborator.build_env_spec Cognitive complexity 37 37
src/hyperloom/orchestrator/actions/executors/baseline.py:2310 BenchmarkRunExecutor._run_once Function length (lines) 683 683
src/hyperloom/orchestrator/actions/executors/baseline.py:3372 BenchmarkRunExecutor._run_single_benchmark Function length (lines) 580 580
src/hyperloom/orchestrator/loop/writeback.py:3528 WritebackCollaborator.build_env_spec Function length (lines) 111 111
src/hyperloom/orchestrator/loop/writeback.py:4566 WritebackCollaborator.validate_geak_via_geak_harness Function length (lines) 187 187
90 findings in files this change does not touch

Outside this change (informational): 90

Unit Dimension Baseline Now
src/hyperloom/orchestrator/enablement/recipe/projections.py project_launch_evidence Cognitive complexity absent 36
src/hyperloom/orchestrator/state/shared_state.py resolve_graded_comparison Cognitive complexity absent 34
src/hyperloom_kb/collect/mapping.py _experience Cognitive complexity absent 31
src/hyperloom_kb/schema.py FieldDeclaration.__post_init__ Cognitive complexity absent 32
src/kernelforge/cli/forge_loop_command.py _prepare_measurement_driver Cognitive complexity absent 36
src/kernelforge/cli/forge_loop_command.py _read_kb_warm_start Cognitive complexity absent 40
src/kernelforge/loop/runner.py IterationLoop._collect_session_candidate Cognitive complexity absent 44
src/kernelforge/loop/runner.py IterationLoop._rule_on_attempt Cognitive complexity absent 38
src/hyperloom_kb/tests/test_cli.py Duplicated lines per file absent 18
src/hyperloom_kb/tests/test_global_identity_and_pull.py Duplicated lines per file absent 13
src/hyperloom_kb/tests/test_local_state.py Duplicated lines per file absent 18
src/hyperloom/inference_optimizer/cli/__init__.py _reexport_session_environment Function length (lines) absent 112
src/hyperloom/inference_optimizer/cli/__init__.py _resolve_agent_lineup Function length (lines) absent 89
src/hyperloom/inference_optimizer/cli/__init__.py _resolve_run_topology Function length (lines) absent 92
src/hyperloom/inference_optimizer/cli/__init__.py _resume_session Function length (lines) absent 171
src/hyperloom/inference_optimizer/cli/__init__.py _seed_fresh_session Function length (lines) absent 183
src/hyperloom/inference_optimizer/multi_node/cli.py _restart_multinode Function length (lines) absent 107
src/hyperloom/inference_optimizer/multi_node/scripts/launch_multinode.py _build_parser Function length (lines) absent 84
src/hyperloom/inference_optimizer/multi_node/scripts/launch_multinode.py _spawn_rank_actors Function length (lines) absent 90
src/hyperloom_kb/http_service.py RequestHandler._dispatch Function length (lines) absent 91
src/hyperloom_kb/sync.py GlobalSync.pull Function length (lines) absent 86
src/kernelforge/cli/forge_loop_command.py _RunReporter.build_result Function length (lines) absent 91
src/kernelforge/cli/forge_loop_command.py _prepare_measurement_driver Function length (lines) absent 140
src/kernelforge/cli/forge_loop_command.py _read_kb_warm_start Function length (lines) absent 154
src/kernelforge/loop/knowledge_integration.py _measure_warmstart_field Function length (lines) absent 83

...and 65 more; the job log lists every row.

Verdict computed by the gate scripts in /home/runner/work/_temp/code-metrics-gate, not by this tree's copy.

Thresholds and sources: [tool.hyperloom.code_metrics] in pyproject.toml. Tools: complexipy 8.0.1, jscpd 5.4.1, ruff 0.16.2, vulture 2.16.

@zoroyihan7
zoroyihan7 force-pushed the fix/agentx-keep-radix-cache branch from 75a644c to 190fe11 Compare October 14, 2026 11:05
An AgentX replay is mostly shared prefix. A baseline served with SGLang's
prefix (radix) cache off measures a cold cache, and any candidate that turns
the cache back on is then credited with the cache itself as a gain.

After a baseline measurement succeeds, the baseline executor now fails it with
error_class=agentx_baseline_prefix_cache_off and stops the run
(stop_reason=baseline_failed) when the replay reports a theoretical prefix hit
of 50% or more (percent or fraction units) and either the server's observed
config has disable_radix_cache=True or every prefill batch in the server log
reused 0 cached tokens. The hit is read from the measurement itself, or from
the raw result at its top level or under request_metrics.cache, so the check
does not depend on which launcher started the server. Candidates are not
judged; a run with no readable evidence is left alone.

Keeping the cache on at launch is left to the launch path.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: zoroyihan7 <Yihan.Wang@amd.com>
@zoroyihan7
zoroyihan7 force-pushed the fix/agentx-keep-radix-cache branch from ecad7cd to 81a69c8 Compare October 14, 2026 12:53
@zoroyihan7 zoroyihan7 changed the title fix(agentx): keep the SGLang prefix cache on for agentic replays and refuse a cacheless baseline fix(agentx): refuse a baseline measured with the prefix cache off Oct 14, 2026
@zoroyihan7 zoroyihan7 added the baseline-raise Waives new, worse and baseline-growth code-metrics findings; reason required in the PR description label Oct 14, 2026

@chaojhou chaojhou left a comment

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.

PR #1884 -- fix(agentx): refuse a baseline measured with the prefix cache off

What it does: An AgentX replay is mostly shared prefix, so a baseline whose server ran with the prefix (radix) cache off measures a cold cache, and every later candidate that turns the cache on is credited with the cache as a gain. After the measurement, BaselineExecutor._run now runs refuse_cacheless_agentx_baseline on genuine baselines: when the client reported a theoretical prefix hit of 50% or more and the server's resolved config shows disable_radix_cache=True (or its SGLang log shows no prefill batch reusing a cached token), the baseline fails with agentx_baseline_prefix_cache_off and the run stops as baseline_failed. Keeping the cache on at launch is left to #1675.

Blocking issues: 3

  1. [C2] The refusal covers SGLang with the aiperf client only; MLPerf-client, vLLM and ATOM AgentX baselines with the cache off are still accepted [verified]
    Problem: prefix_cache_off_evidence returns "" when the client reported no hit, before it reads the server config (src/hyperloom/orchestrator/actions/executors/_agentx_prefix_cache.py:86-88), and it reads only SGLang's disable_radix_cache (:91) and SGLang's #cached-token: log line (:23). The MLPerf agentic client (HYPERLOOM_AGENTIC_BACKEND=mlperf, a documented AgentX backend on the same Magpie launcher) writes no hit (map_mlperf, agentx/mapping.py:262-320); vLLM's launch record keeps every non-default key (common/launch_log_evidence.py:333-343), so --no-enable-prefix-caching is observed as enable_prefix_caching: False, which is not read; ATOM's cache is off unless --enable_prefix_caching is passed (its seed grid ships that flag as the atom_prefix_cache candidate, executors/explore.py:347) and ATOM has no launch record. AgentX runs on every serving framework (apply_agentx_switch skips only scriptable ones, executors/_workload_envs.py:562-564; atom is a serving framework in inference_optimizer/framework_registry.py). src/hyperloom/inference_optimizer/SKILL.md:989-994 says such a baseline stops the run whichever launcher started the server, and the description says "An AgentX run whose baseline server has the prefix cache off now stops at the baseline". src/hyperloom/orchestrator/actions/executors/_agentx_prefix_cache.py:86
    Impact: on those configurations the cold-cache baseline is still the anchor, so a candidate that turns the cache on (on ATOM, the seed grid's own atom_prefix_cache) is credited with the cache as a gain -- the inflation this PR exists to stop -- while the agent is told the run would have stopped.
    Action: gate the check on agentx_active(shared_state) rather than a client-reported hit, read each engine's own setting from launch_evidence.observed_server_config (SGLang disable_radix_cache, vLLM enable_prefix_caching), and turn ATOM's cache on at launch since it records no config; or narrow the title, the SKILL.md paragraph and the description to SGLang with the aiperf client and name the uncovered paths.
  2. [C3] Two of the three places the theoretical hit is read have no writer at this head, and the unit guess that reconciles them misreads the one producer that exists [verified]
    Problem: _theoretical_prefix_hit_pct reads a result-level theoretical_prefix_cache_hit (src/hyperloom/orchestrator/actions/executors/_agentx_prefix_cache.py:44) and request_metrics.cache.theoretical_cache_hit_rate from the raw file (:59-61); nothing in the head tree writes either (benchmark_result._merge_raw_result copies neither, and request_metrics occurs only in this module and its test); their only producer is #1675's native path, which is still open. _as_pct (:26-35) then guesses the unit from the magnitude to reconcile #1675's fraction with aiperf's percent, so the in-tree producer's percent (map_aiperf, agentx/mapping.py:150; {"unit": "%", "avg": 0.73} in test_map_aiperf.py:46) is multiplied by 100 whenever it is 1 or less. src/hyperloom/orchestrator/actions/executors/_agentx_prefix_cache.py:59
    Impact: two of the three reads can never fire at this head, and they encode an unmerged PR's output shape that nothing here produces; the unit guess reads an aiperf hit between 0.5% and 1% as 50-100%, so a cache-off baseline on such a corpus would be refused and the run stopped (no in-tree corpus sits there today, so this part is latent).
    Action: read the hit only where the in-tree producer writes it (aiperf's mapped export, in percent) and delete the result-level and request_metrics.cache reads and _as_pct, leaving #1675 to add its own shape when it lands; or, with the check gated on agentx_active(shared_state) (item 1), delete the hit reader, _as_pct and PREFIX_HEAVY_HIT_PCT altogether.
  3. [A13] A type re-check that cannot fail [verified]
    Problem: src/hyperloom/orchestrator/actions/executors/_agentx_prefix_cache.py:89 assigns evidence from ... if isinstance(..., Mapping) else {}, so isinstance(evidence, Mapping) at :90 is true for every input and its else None arm is dead. AGENTS.md Trust the caller: "redundant re-checks, layered fallbacks, and belt-and-braces defaults buy nothing". src/hyperloom/orchestrator/actions/executors/_agentx_prefix_cache.py:90
    Impact: a dead branch in the function every genuine baseline passes through.
    Action: read evidence.get("observed_server_config") directly at :90.

Checked: _agentx_prefix_cache.py (all); executors/baseline.py (BaselineExecutor._run, _run_retrying, _should_establish_quality_ref, launch-evidence attachment); benchmark_result.py (extract_benchmark_measurement, _merge_raw_result); common/launch_log_evidence.py (_LAUNCH_RECORDS, observed_server_config_from_log); agentx/mapping.py (map_aiperf, map_mlperf); agentx/preflight.py; assets/agentx aiperf and mlperf clients; grid_server_args.py; explore.py (ATOM seed grid); _workload_envs.agentx_active; shared_state set_stop_reason; SKILL.md; #1675's benchmark_result.py diff; tree-wide sweep at head for every writer and reader of theoretical_prefix_cache_hit, theoretical_cache_hit_rate, request_metrics, observed_server_config, disable_radix_cache, enable_prefix_caching and #cached-token | Ran: direct calls into the head prefix_cache_off_evidence, map_aiperf/map_mlperf and observed_server_config_from_log on SGLang, MLPerf, vLLM, ATOM and unit-boundary cases; python ..\pr_tools\c901.py on the two changed source files (ruff C901 counting; ruff is not installed here): new functions 2-6, BaselineExecutor._run 4 -> 5; gh run list --commit 81a69c87 (latest run of each workflow: 10 success, 2 label-gated E2E skipped); gh api .../pulls/<n>/files --paginate for the partially listed open PRs (at most 2 of the 4 files overlap); the PR's own tests were not run locally | Base: 190604f | Head: 81a69c8

zoroyihan7 and others added 2 commits October 15, 2026 03:33
…tting

Address review on #1884:
- Gate the refusal on agentx_active(shared_state) instead of a
  client-reported theoretical prefix hit, so the MLPerf client (which
  reports no hit) is covered too. This removes the hit reader, its unit
  guess and the 50% threshold.
- Read each engine's own setting: SGLang disable_radix_cache (or a log
  whose prefill batches never reused a cached token), vLLM
  enable_prefix_caching=False from its launch record, and ATOM's
  requested server args, since ATOM keeps the cache off by default and
  prints no launch record.
- Drop the type re-check that could not fail.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: zoroyihan7 <Yihan.Wang@amd.com>
…efill log

A server whose resolved config says disable_radix_cache=False has the
cache on even when no prefill batch found anything to reuse (cold
requests, eviction), so the prefill log is read only when the server
printed no resolved setting. Pin that ATOM's check reads the server args
the materialized config launches with.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: zoroyihan7 <Yihan.Wang@amd.com>
@zoroyihan7

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. Addressed in 41ad6e3 and 20891e2 (new commits on top of 81a69c8, no force-push).

  1. [C2] Only SGLang + aiperf covered. Fixed, taking the first option. The refusal is now gated on agentx_active(shared_state) instead of a client-reported hit, so the MLPerf client is covered. Each engine is read from its own launch account in launch_evidence: SGLang observed_server_config.disable_radix_cache=True (the prefill-log check is now only a fallback when the server printed no resolved setting, so a cache-on server that reused nothing is not refused); vLLM observed_server_config.enable_prefix_caching=False (tested through observed_server_config_from_log, with the absent key read as the default, on); ATOM has no launch record and defaults to off, so it is refused when requested_server_args (the materialized EXTRA_ATOM_ARGS, which Magpie's ATOM launcher passes as is and adds no prefix flag to) lacks --enable_prefix_caching. I did not turn ATOM's cache on at launch: this PR stays a baseline guard and launch changes are feat(agentx) Use Magpie native AgentX for Hyperloom optimization #1675's; the refusal message tells the operator to pass --server-args "--enable_prefix_caching". SKILL.md and the description now say exactly this.
  2. [C3] Hit readers with no in-tree writer, and the unit guess. Fixed by removing them: with the AgentX gate, the hit reader, _as_pct and PREFIX_HEAVY_HIT_PCT are deleted, along with the result-level and request_metrics.cache reads.
  3. [A13] Dead type re-check. Fixed: prefix_cache_off_evidence returns early when launch_evidence is not a mapping and then reads observed_server_config directly.

Tests: test_agentx_prefix_cache.py 28 passed (per-engine matrix, ATOM through build_launch_evidence from a materialized config, AgentX by state or by env, synthetic baseline not judged); 7 mutants (AgentX gate, vLLM check, ATOM refusal, ATOM flag spelling, SGLang setting-before-log precedence, cached-token reuse, ATOM args source) each fail at least one test. ruff check / ruff format --check clean.

Comment thread src/hyperloom/orchestrator/actions/executors/baseline.py Fixed
@zoroyihan7
zoroyihan7 requested a review from chaojhou October 15, 2026 04:34
CodeQL flagged the module-level import as the start of an import cycle
through baseline. Import it where it is used instead; behaviour is
unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: zoroyihan7 <Yihan.Wang@amd.com>
CodeQL counts function-local imports too, so moving the import into the
function only moved the cyclic-import alert. Read the predicate from
hyperloom.common.perf_metric, which _workload_envs.agentx_active
delegates to with the same benchmark_mode argument, so the prefix-cache
check no longer imports anything from the executors package.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: zoroyihan7 <Yihan.Wang@amd.com>

@chaojhou chaojhou left a comment

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.

PR #1884 -- fix(agentx): refuse a baseline measured with the prefix cache off

What it does: An AgentX replay is mostly shared prefix, so a baseline measured with the server's prefix cache off is cold, and any candidate that turns the cache on is credited with the cache as a gain. After a genuine baseline is measured, BaselineExecutor._run now calls refuse_cacheless_agentx_baseline when agentx_active holds. It reads each engine's own launch evidence: SGLang's resolved disable_radix_cache, falling back to the server log's #cached-token: counts when no resolved setting was read; vLLM's enable_prefix_caching=False; and, for ATOM, --enable_prefix_caching missing from the requested server args. When the cache was off, it fails the baseline with agentx_baseline_prefix_cache_off and sets stop_reason=baseline_failed. The previous card's three items (engine/client coverage, hit readers with no writer, dead type re-check) are fixed at this head.

Blocking issues: 2

  1. [D1] The refusal makes its own stop decision in the executor, and writeback, which has no branch for the new class, records it as a boot failure that opens enablement [verified]
    Problem: refuse_cacheless_agentx_baseline calls shared_state.set_stop_reason("baseline_failed") itself (src/hyperloom/orchestrator/actions/executors/_agentx_prefix_cache.py:103). loop/writeback.py is unchanged and never names agentx_baseline_prefix_cache_off, so the refused result takes the generic else branch (writeback.py:1299). _authorable_launch_log excludes only fast_exit_arg_error and AGENTX_PREFLIGHT_ERROR_CLASS (writeback.py:130), so it returns the refusal text. writeback.py:1333-1344 then records it via enablement.record_launch_observation and enablement_event.record_trigger(origin=ORIGIN_BOOT, kind="agentx_baseline_prefix_cache_off"). writeback already owns this decision for the closest precedent, AGENTX_PREFLIGHT_ERROR_CLASS (writeback.py:1277-1298), whose comment records that routing it as a launch failure opened an enablement round. AGENTS.md Clean design: "One boundary rule per concern, owned by one module ... duplicated state or a duplicated decision is the same problem". Owner: orchestrator/loop/writeback.py. src/hyperloom/orchestrator/actions/executors/_agentx_prefix_cache.py:103
    Impact: every refused AgentX baseline leaves a persisted enablement event saying a boot failure opened the repair lane, for a server that booted and measured fine. It also leaves the refusal text in enablement state as a launch log to author a framework patch against. The run still stops, but its record is wrong.
    Action: have the executor only mark the result failed with PREFIX_CACHE_OFF_ERROR_CLASS. In writeback, add a branch beside AGENTX_PREFLIGHT_ERROR_CLASS that stops on the first occurrence, and add the class to _authorable_launch_log's exclusion tuple. This deletes the shared_state lookup, the if shared_state is not None arm and the stop call from the new module. Move the "stops the run" assertion to one writeback-level test that also asserts no enablement trigger or launch observation is recorded.
  2. [A13] The SGLang #cached-token log scan is a layered fallback behind the authoritative record, with a different rule [verified]
    Problem: SGLang's server_args record is read whole (_LAUNCH_RECORDS["sglang"], common/launch_log_evidence.py), so disable_radix_cache is present whenever the record was read. The fallback (_agentx_prefix_cache.py:20, :26-38, :46-47) therefore runs only when that record could not be read: no record, record past the line/char cap, or unparseable. There it decides by behaviour rather than configuration, and refuses a server whose batches all reused 0 tokens. That is the "cache on but found nothing to reuse" case the PR's own test keeps when the setting is readable (test_agentx_prefix_cache.py:74-75). vLLM has no such fallback, and the module docstring says "A run none of these can be read from is left alone". This full-log scan is the check's only file I/O and the only reason baseline.py:4214 wraps the call in asyncio.to_thread. AGENTS.md Trust the caller: "layered fallbacks ... buy nothing -- they hide the failure they were added to survive". src/hyperloom/orchestrator/actions/executors/_agentx_prefix_cache.py:46
    Impact: one more mechanism, a second copy of the cache-off decision whose rule disagrees with the first, and a threaded full-log scan, all for a state the module's own docstring says to leave alone.
    Action: delete _CACHED_TOKEN_RE, _server_reused_cached_tokens and the fallback branch, so an unreadable SGLang record is left alone as vLLM's is. Delete the fallback test rows and the SKILL.md/description clause "or prefill batches that never reused a cached token". Call the check inline instead of through asyncio.to_thread.

Checked: _agentx_prefix_cache.py (all); baseline.py (BaselineExecutor._run, _run_retrying, _maybe_stop_on_missing_baseline_accuracy, _should_establish_quality_ref, _attach_baseline_launch_evidence); loop/writeback.py (_authorable_launch_log, the baseline-failure branches of handle_unpromotable_result, _generic_baseline_failure_stops); enablement_event.record_trigger; common/launch_log_evidence.py (_LAUNCH_RECORDS, observed_server_config_from_log); perf_metric.agentx_active; _workload_envs.apply_agentx_switch; _multi_node_server_lifecycle._SGLANG_DEFAULT_TOKENS; Magpie upstream scripts/benchmark/sglang_mi300x.sh and sglang_mi355x.sh (DEFAULT_ARGS); explore.py ATOM seed grid; SKILL.md; the test file; git grep at head for the new error class, disable-radix-cache, enable_prefix_caching, set_stop_reason | Ran: git diff 81a69c87b 27124c1ee; head observed_server_config_from_log on readable and unreadable SGLang ServerArgs records; two independent refutation passes per finding; the PR's tests were not run locally | Base: 190604f | Head: 27124c1

zoroyihan7 and others added 3 commits October 15, 2026 09:38
The executor now only marks the refused baseline failed with
agentx_baseline_prefix_cache_off. Writeback stops the run on its first
occurrence (beside ARGV_INVALID in one first-occurrence table) and
_authorable_launch_log excludes the class, so the refusal no longer
records a boot trigger or a launch observation for enablement.

Drop the SGLang #cached-token log fallback: an unreadable ServerArgs
record is left alone, as an unread vLLM record is, and the check runs
inline instead of in a thread.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: zoroyihan7 <Yihan.Wang@amd.com>
is_promotable_result judged a baseline by its measurement alone, so a
round refused for a cold prefix cache, which still carries throughput
and completed requests, would have been promoted as the anchor and
never reached handle_unpromotable_result. Treat the refusal class like
an eval-failed baseline there.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: zoroyihan7 <Yihan.Wang@amd.com>
handle_unpromotable_result runs its baseline gates only while no
baseline has succeeded, so a genuine re-baseline refused for a cold
prefix cache after an anchor existed was neither promoted nor stopped.
The cache-off class now passes those gates whatever the anchor, the
way the executor-side stop applied before.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: zoroyihan7 <Yihan.Wang@amd.com>
@zoroyihan7

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. Both items are addressed in be7db3f. Tracing D1 through the dispatcher turned up two more defects in the same path, fixed in f040f87 and 9e60e6b.

1. [D1] Stop decision owned by writeback; refusal kept out of enablement. Fixed.

  • refuse_cacheless_agentx_baseline(result) now only marks the result status=failed, error_class=agentx_baseline_prefix_cache_off. The shared_state lookup, the if shared_state is not None arm and the set_stop_reason call are gone from the module; the agentx_active(shared_state) gate sits with the existing _should_establish_quality_ref gate at the call site in BaselineExecutor._run.
  • loop/writeback.py: handle_unpromotable_result's ARGV_INVALID branch became a lookup in a module-level _STOPS_ON_FIRST_OCCURRENCE table (ARGV_INVALID -> ARGV_INVALID, PREFIX_CACHE_OFF_ERROR_CLASS -> "baseline_failed"), so the new class stops on the first occurrence with no streak, and ARGV_INVALID behaves exactly as before. I used the table rather than a fourth elif so handle_unpromotable_result, already in the code-metrics baseline, does not grow (C901 25 and cognitive 55, same as before; 2 lines shorter). _authorable_launch_log excludes the class beside fast_exit_arg_error and AGENTX_PREFLIGHT_ERROR_CLASS.
  • Additional defect, same path (f040f87): the dispatcher only calls handle_unpromotable_result when is_promotable_result says no, and for a baseline that was is_valid_measurement(result) alone, which reads throughput and completed requests and ignores status. A refused round keeps both, so on the real path it would have been promoted as the anchor, and writeback's new stop would never have run. That was also true at the previous heads, where the executor-side stop ended the run but the cold baseline had already become the anchor. is_promotable_result now returns False for this class, next to the existing baseline_eval_failed rule.
  • Test: the "stops the run" assertion moved to one writeback-level test with enablement on (enablement_mode="all"), parametrized over the first baseline and a re-baseline after an anchor (baseline_tput=200). A refused round with real throughput (completed_requests=64) is not promotable while a cache-on one is. handle_unpromotable_result sets stop_reason=baseline_failed, records no enablement_event.record_trigger, stashes no launch log, leaves enablement.origin empty and leaves the failure streak at 0. The executor-level tests now assert the result is marked and stop_reason is still unset. Before the writeback change the test fails (stop_reason == ''). It also fails under each of these mutants: the class dropped from _authorable_launch_log's exclusion (a trigger gets recorded), the class dropped from is_promotable_result (the round is promotable), the AgentX gate removed from _run (test_a_synthetic_baseline_is_not_judged), and the class dropped from _arms_baseline_failure_gates (the re-baseline case does not stop).
  • Additional defect (9e60e6b): handle_unpromotable_result applies its baseline gates only while no baseline has succeeded (baseline_tput <= 0). Moving the stop there would have dropped the old executor-side stop for a genuine re-baseline refused after an anchor exists, for example an enablement revalidation. A small _arms_baseline_failure_gates(err_class) predicate now lets this class through the gates whatever the anchor. handle_unpromotable_result keeps C901 25 and cognitive 55 and is 2 lines shorter than on main.

2. [A13] #cached-token fallback removed. Fixed. I deleted _CACHED_TOKEN_RE, _server_reused_cached_tokens and the fallback branch. SGLang now refuses only on a resolved disable_radix_cache=True, and an unread ServerArgs record is left alone, the same as vLLM. The check runs inline, without asyncio.to_thread. Its fallback test rows are gone. I added one row that reads disable_radix_cache through the real observed_server_config_from_log parser, and the "no resolved setting" row now expects the run to be left alone. I dropped the clause from SKILL.md, which now says an unreadable launch account is left alone, and from the description.

CodeQL cyclic-import alerts (#3791/#3792/#3793). These are still fixed at this head: _agentx_prefix_cache no longer imports from the executors package at all (the agentx_active gate moved to the caller), and the code-scanning API shows 0 open alerts on refs/pull/1884/merge.

Ran: the PR's test file (26 passed). Every test file that references is_promotable_result, handle_unpromotable_result, _authorable_launch_log, ARGV_INVALID, baseline_eval_failed, baseline_tput or the guard: 5219 passed. Three failed: test_trace_analyze_handler_dry_run_returns_structured_result, test_enablement_zero_patch_round_does_not_erase_prior_accepted_work and test_credentials_validate_and_reset_claude_config, all of which fail the same way on the previous head in my environment. Every test file that references BaselineExecutor, _workload_envs, agentx_active or launch_log_evidence: all passed except test_trace_analyze_handler_dry_run_returns_structured_result, which fails the same way on the previous head in my environment. ruff check and ruff format --check are clean. The description is updated.

@zoroyihan7
zoroyihan7 requested a review from chaojhou October 15, 2026 11:23
The code-metrics gate counted 'baseline_failed' three times in
writeback.py after the prefix-cache-off class joined
_STOPS_ON_FIRST_OCCURRENCE. Name it as a module-level constant, and
lower handle_unpromotable_result's recorded length (233 -> 231).

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: zoroyihan7 <Yihan.Wang@amd.com>
@zoroyihan7

Copy link
Copy Markdown
Collaborator Author

Follow-up at d0c8379: the code-metrics gate on 9e60e6b failed on one repeated literal, 'baseline_failed' written three times in loop/writeback.py once the new class joined _STOPS_ON_FIRST_OCCURRENCE. It is now named once as the module-level _BASELINE_FAILED_STOP_REASON, used by the table and both set_stop_reason calls. No behaviour change: the module's AST is identical to the previous head's once the constant is inlined. I also lowered the baseline entry for handle_unpromotable_result (233 -> 231), as the gate's "baseline out of date" row asked.

CodeQL #3793 (and #3791/#3792) remain fixed at this head: _agentx_prefix_cache imports nothing from the executors package, and code scanning shows 0 open alerts on refs/pull/1884/merge.

Ran: writeback/baseline-failure test files (421 passed), ruff check/ruff format --check clean, code-metrics locally (repeated literals 0). CI at d0c8379: 33 passed, 7 skipped, 0 failed, code-metrics included.

@chaojhou
chaojhou dismissed stale reviews from themself October 16, 2026 14:30

Superseded: re-reviewed at d0c8379, both items fixed; no blocking issues.

@chaojhou

Copy link
Copy Markdown
Collaborator

PR #1884 -- fix(agentx): refuse a baseline measured with the prefix cache off

What it does: An AgentX replay is mostly shared prefix, so a baseline measured with the server's prefix cache off is cold, and any candidate that turns the cache on is credited with the cache as a gain. A genuine AgentX baseline is now marked failed with agentx_baseline_prefix_cache_off when its engine's own launch account says the cache was off (SGLang's resolved disable_radix_cache=True, vLLM's enable_prefix_caching=False, ATOM server args without --enable_prefix_caching); a launch whose account cannot be read is left alone. Writeback owns what follows: is_promotable_result refuses the result so it can never become the anchor, handle_unpromotable_result stops the run on the first occurrence through _STOPS_ON_FIRST_OCCURRENCE (also for a re-baseline refused after an anchor exists), and _authorable_launch_log keeps the refusal out of the enablement lane. Both items on the previous card are fixed at this head: the executor no longer sets the stop reason, and the #cached-token fallback and its asyncio.to_thread call are gone. Every path that can make a baseline the anchor runs through dispatcher.reap_dispatched_task -> is_promotable_result -> _promote_baseline, so a refused round cannot anchor anywhere, and ARGV_INVALID behaves exactly as before the table refactor.

Blocking issues: none

Checked: _agentx_prefix_cache.py (all); baseline.py BaselineExecutor._run and the _workload_envs.agentx_active it calls; writeback.py (_STOPS_ON_FIRST_OCCURRENCE, _authorable_launch_log, is_promotable_result, _arms_baseline_failure_gates, the baseline gate block of handle_unpromotable_result including _record_revalidation_not_promoted, _promote_baseline); dispatcher.py reap_dispatched_task and the dead-holder reclaim; every caller of is_promotable_result, handle_unpromotable_result, promote_to_shared_state and every writer of baseline_tput; kernel.py, prelude.py and request_handlers.py paths that read is_valid_measurement; SKILL.md; the test file; the description | Ran: the head writeback flow on constructed results on a real Coordinator (refused first baseline, refused re-baseline with an anchor of 200, enablement revalidation with validation_pending, ARGV_INVALID with and without an anchor); git diff 27124c1ee d0c83790d; CI at d0c8379 (33 pass, 7 label-gated skipped). Not run locally: the PR's pytest file (no pytest in this environment) | Base: 190604f | Head: d0c8379

LGTM

@chaojhou
chaojhou merged commit cd9eebb into main Oct 16, 2026
40 checks passed
@chaojhou
chaojhou deleted the fix/agentx-keep-radix-cache branch October 16, 2026 14:30
zoroyihan7 added a commit that referenced this pull request Oct 16, 2026
…ache off succeeded (#1884)

The hosted executor reads the launch of each measured baseline round of an
AgentX campaign through main's refuse_cacheless_agentx_baseline and ends the
test as a candidate failure named agentx_baseline_prefix_cache_off, so
establish_baseline never counts it.

Interface-Change: https://global.primus-safe.amd.com/board/boards/hyperloom-multibranch/issues/48

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

baseline-raise Waives new, worse and baseline-growth code-metrics findings; reason required in the PR description domain:inference Related to inference optimization type:bug Something is broken or not working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants