feat(e2e): read the Architect's "drop list" instead of discarding it - #412
Draft
sdubagun-amd wants to merge 3 commits into
Draft
feat(e2e): read the Architect's "drop list" instead of discarding it#412sdubagun-amd wants to merge 3 commits into
sdubagun-amd wants to merge 3 commits into
Conversation
The System Architect has always returned a `drop_list` — the candidates it
judged not worth the time, with reasons — and nothing has ever read it: not
code, not a log, not a report, not another role's prompt, not saved state.
This wires it up behind a flag, and folds three latent bugs into the same fix.
New `normalizeQueues({head, kernel, dropList, origin})`, called at all three
places the work queues are assigned (initial strategize, resume from carried
state, post-config re-strategize). It does three things in order:
1. Identity. Fill in a missing `id` only, and mark it `id_synthesized` so a
drop_list entry can never match a name we invented ourselves. `short_name`
is deliberately left alone — the head and milestone tracks generate their
own names downstream and those reach reports and task directories.
2. Op-identity guard, moved here from the single post-strategize site. It now
runs after a re-plan too (bug 8: a re-strategize replaced the queue with
fresh untagged candidates, so the "a fused kernel is never SKIPPED"
protection silently disappeared), and it binds any head with a live call
seam, not just fused ones (bug 7).
3. Relevance drop, gated by the new `drop_gate` setting:
off ignore drop_list entirely; queues are exactly what the Architect returned
dryrun match and log what WOULD be dropped, drop nothing (default)
on actually drop
Default is dryrun because this list has never been acted on: the first
runs prove the matching is sound before it is allowed to remove work.
A candidate at or above HEAD_PROTECT_PCT is refused in EVERY mode. A
drop_list entry that matches nothing is logged loudly and recorded, since
a silent no-match looks identical to a working filter.
Inputs are deep-copied, so the flydsl strip and the op-identity guard no longer
write through into the Architect's own candidate objects and no longer persist
those mutations into carried state (bug 6).
Every decision is recorded in `drop_decisions`, persisted into carry state and
the workflow return, and summarised unconditionally at the end of the run (not
inside the head-track branch — drops affect the milestone queue too).
roles/system_architect.md now asks for an `id` and `pct_gpu_time` on each
drop_list entry, so matching has something reliable to key on. No schema gained
a `required` entry: obj() emits additionalProperties:true and nothing validates
locally, so a `required` entry would change LLM generation on every run
regardless of the flag, breaking "off behaves like today".
Tests: e2e_workflow/scripts/test_drop_gate.js extracts the real
normalizeQueues block from the orchestrator and runs it with controlled deps,
asserting off is inert, dryrun records without dropping, protection holds in
every mode, an invented id cannot match, seam matching ignores the advisory
signature, inputs are not mutated, the function is idempotent, all three call
sites route through it, and no schema gained a required entry.
CI: the node-regression job now also runs `node --check` on the orchestrator
and this new test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…fusal checks A gate that is inert by default changes nothing on merge. Flip the default to `on` and remove `dryrun`, which earned nothing: logging was already identical in `on`, so it was an observe-only mode with nothing to observe that `on` does not also print. `off` stays as a kill switch. Three real risks stood between `dryrun` and an on-by-default gate. All three are now closed inside normalizeQueues step (3), which became a two-pass resolve-then-commit so the whole list can be weighed before any of it applies: 1. A missing GPU-time share read as 0%. `Number(c.pct_gpu_time) || 0` maps an absent field to the single most droppable value there is, sailing straight past the HEAD_PROTECT_PCT floor. New `_pct` helper returns null for absent/blank/NaN and keeps an explicit 0 as 0; an unsizeable candidate is refused as `unverified`. The drop entry's own pct_gpu_time is accepted as a fallback before giving up. 2. Nothing bounded a bulk drop. A drop_list covering the queue would have emptied it. New DROP_MAX_FRACTION (0.5, a module const, not a tuning knob): a list resolving to more than half the pool is refused WHOLE and every entry recorded as `refused_bulk`, rather than an arbitrary half being applied. 3. Matching fell back silently. An entry with a stale or hallucinated id would miss on id and then land on some other candidate by short_name. An entry carrying an id now matches on that id ALONE, and an entry resolving to two candidates is refused as `ambiguous` rather than guessed at. So a drop now needs all four: exactly one match, a known size, a size under HEAD_PROTECT_PCT, and a list that is not eating the queue. Every refusal is logged with the Architect's stated reason and carried in drop_decisions. test_drop_gate.js: dryrun scenarios removed, scenarios added for unverified (undefined/null/''/NaN, plus explicit-0 IS droppable and entry-supplied size), ambiguous, refused_bulk, at-the-cap, and id-no-fallback; plus source assertions pinning the default to 'on', the absence of dryrun, and DROP_MAX_FRACTION. Verified locally without a JS runtime: delimiter balance on both files, all 24 source-level assertions evaluated directly against the sources, and a Python transliteration of normalizeQueues passing all 39 behavioural scenarios. CI runs the real node. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…un mode The step comment claimed the test proves "dryrun records but drops nothing" and that protection holds "in any mode". dryrun no longer exists and there are now four refusal checks, not one. Describe what the test actually proves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
When GEAK starts a run, one of its agents — the System Architect — reads the GPU profile and
decides what to work on. It produces two things:
(
"below Amdahl threshold","too small to move e2e", and so on).GEAK has always acted on the first list and thrown the second one away. Nothing reads
it — not the orchestrator, not a log line, not a report, not another
agent's prompt, not the saved state that survives a resume.
So the planner identifies the wasted work, says so out loud, and then the run goes and does it
anyway. In the 15-hour Qwen3-14B-FP8 baseline the string
drop_listappears zero times in theentire run tree. It was written and immediately discarded.
What this PR changes
GEAK now reads that list.
Concretely: there are three points in a run where the work list gets set — when the Architect first
plans, when a run resumes from saved state, and again after the config sweep, when the Architect
re-plans with fresh information. Each of those three places used to just take the agent's output
as-is.
This PR puts a single shared cleanup step in front of all three, a function
normalizeQueues.The cleanup step does three things, in this order:
First, make sure every kernel can be referred to
A drop entry says "skip this one", which only works if both lists agree on which one. The
Architect returns each candidate as a free-form JSON object and the schema requires no fields at
all, so a candidate can arrive as just
{"short_name": "rmsnorm", "pct_gpu_time": 2.0}— with noidentifier. For those we fill in a positional stand-in (
h0,h1,k0, …) so every candidate hasa handle for the rest of the run.
We also record that we invented it. An id GEAK assigned is not something the Architect could
ever have written down, so a drop entry naming
h3must never match a candidate whoseh3wemade up.
Second, re-apply the fused-kernel protection
A Mixture-of-Experts layer routes each token to a few of many small expert matrices. A fused MoE
kernel does all of that in one GPU launch. GEAK has a rule that one of those must never be treated
as an ordinary dense matrix multiply. In code the rule does two things: it tags the candidate as
moe, which stops GEAK generating a dense matrix-multiply benchmark for it, and it holds onto thePython function where the fused kernel is actually called, which is the only place a replacement
can be installed. A dense matrix multiply has no such call site — so a fused kernel mistaken for a
dense one ends the run with a result that cannot be installed anywhere.
Why it needs to survive a re-plan. Of the three points where the work list is set, this rule
used to run at only the first. After the config sweep the Architect re-plans, and the code
replaced the whole work list with brand-new candidate objects that had never been through the
rule — tag gone, call site gone, nothing to re-apply them. Moving the rule into the shared step
means all three points get it: first plan, resume, and re-plan. That is bug 1 below.
Third, apply the drop list
Match each rejected entry against the work list and remove it — but only when the match is
unambiguous and the drop is justified, which is the next section. If the entry carries the
Architect's own identifier we match on that and nothing else. If it does not, we fall back to the
kernel's short name, or to the Python function GEAK would have to swap out to replace it (written
as
module:function) — whichever the agent gave us.The
drop_gatesettingThe default is
on. Merging this changes what runs do.onoffThe first two steps above — filling in identifiers and the fused-kernel protection — run in
both settings. They are bug fixes, not part of the gate.
What makes
onsafe as a defaultA drop is refused unless all four of these hold. Every refusal is logged with the Architect's
own stated reason, saved into the state that survives a resume, and counted in a one-line summary
at the end of the run.
Exactly one match. If the entry carries an identifier, it is matched on that identifier
alone — no falling back to the name. Otherwise a stale or hallucinated identifier would miss,
then quietly land on some other kernel by name. An entry that matches two kernels is refused
rather than guessed at; a drop is not reversible.
A known size. The entire argument for a drop is "too small to be worth the time", so a
kernel whose GPU-time share we cannot read is one we cannot justify dropping. This is worth
spelling out because the obvious code is wrong here:
Number(x) || 0turns a missing field into0%— the single most droppable value there is — and would walk straight past check 3. Missing,blank or unparseable is refused. An explicit
0is a real measurement and is allowed.Under 30% of GPU time (
HEAD_PROTECT_PCT). The dominant kernel in the profile is neverdropped on an agent's say-so.
The list is not eating the queue. A drop list resolving to more than half the work list
(
DROP_MAX_FRACTION) is refused whole — not half-applied in arbitrary order. At that size theuseful signal is "the plan is wrong", not "here are some kernels".
A drop entry that matches nothing is logged loudly and recorded too. A silent no-match looks
exactly like a filter that is working.
Three bugs fixed along the way
These are independent of the setting above and are fixed in both settings.
A re-plan silently dropped the fused-kernel protection. The rule that keeps a fused
Mixture-of-Experts kernel intact ran only after the first plan. The config sweep's re-plan
replaced the work list with fresh, untagged candidates and the rule never ran again. Its own
comment in the source says a fused kernel "is never SKIPPED" — after a re-plan, it could be.
Only fused kernels got wired to their call site. The same loop that applied the fused rule
was also the only place that recorded which Python function to swap out for a kernel, and it
skipped everything that was not fused. Now every candidate that has one gets it.
The work list was copied shallowly.
.slice()copies the array but not the objects insideit, so later steps were writing into the Architect's own returned objects, and those edits were
being saved into the state that survives a resume. The shared step now takes a deep copy.
Tests
e2e_workflow/scripts/test_drop_gate.jspulls the real cleanup function out of the orchestratorsource and runs it against a synthetic work list. 54 assertions, covering:
offis inert, andonis the shipped default;0is allowed through;
(which a resume or a re-plan does);
🤖 Generated with Claude Code and edited manually.