【WIP】refactor(kernel): one session view, one lane shape, one integration entry - #1465
Draft
jiaqiang-dot-liu wants to merge 11 commits into
Draft
jiaqiang-dot-liu wants to merge 11 commits into
jiaqiang-dot-liu wants to merge 11 commits into
Conversation
request_handlers is the dispatch table, three lane drivers, the shape materialization and the integrate path all at once. The scanners that answer "what did this session already produce" are none of those: which serving log carries aiter dispatch evidence, which trace the fusion discover stage can read, which untuned CSV belongs to this model. They are reads, and every lane needs the same answers, so they belong in a module a lane can import rather than in the one lane that happened to need them first. kernel_evidence collects them. The six names a caller outside the module uses lose their leading underscore, because a module-private spelling on a cross-module import says the opposite of what is true. The rest stay private. The split runs through the tests too. The server-log suite was two suites in one file -- the scanners, and what the GEMM lane forwards once they have answered -- so each half now sits with the module it pins. The three server-log cases in the units file are dropped rather than moved: the evidence suite already covers each of them and additionally requires the dispatch evidence they predate. No behaviour change.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| monkeypatch.setattr(rh, "_forge_gemm_tune_available", lambda: True) | ||
| monkeypatch.setattr(rh, "_resolve_forge_precision_and_quant", lambda *_a, **_k: ("bf16", "")) | ||
| monkeypatch.setattr(model_paths, "resolve_serving_model_path", lambda p: str(p)) |
|
|
||
| monkeypatch.setattr(rh, "_forge_gemm_tune_available", lambda: True) | ||
| monkeypatch.setattr(rh, "_resolve_forge_precision_and_quant", lambda *_a, **_k: ("bf16", "")) | ||
| monkeypatch.setattr(model_paths, "resolve_serving_model_path", lambda p: str(p)) |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| monkeypatch.setattr(rh, "_forge_gemm_tune_available", lambda: True) | ||
| monkeypatch.setattr(rh, "_resolve_forge_precision_and_quant", lambda *_a, **_k: ("bf16", "")) | ||
| monkeypatch.setattr(model_paths, "resolve_serving_model_path", lambda p: str(p)) |
|
|
||
| monkeypatch.setattr(rh, "_forge_gemm_tune_available", lambda: True) | ||
| monkeypatch.setattr(rh, "_resolve_forge_precision_and_quant", lambda *_a, **_k: ("bf16", "")) | ||
| monkeypatch.setattr(model_paths, "resolve_serving_model_path", lambda p: str(p)) |
KernelContext names what a lane is handed: workload, serving surface, evidence index. Three sources in a fixed order -- the request payload, the live profile context, then SharedState -- and no fourth. The environment is not a source here: SharedState already carries tp, conc, gpu_type and the rest, and the second reader is how the lanes came to disagree. One builder, not one instance. A lane KEEPs, current_best moves, and the next lane has to describe the stack it will actually measure against, so a lane builds its own context immediately before it runs. What is shared is the code path, which is where the divergence lived. Discovery only, and the line is load-bearing rather than stylistic: every field is a read, so a context can be built anywhere without booting a server or touching a GPU. Capturing shapes, writing a CSV and sealing a baseline all mutate something, so they stay in the lane and reach a projection as an argument. Two shapes carry an answer a bare string cannot. ArtifactRef separates "nobody produced this" from "the producer named a path that is gone", which want different next steps. An unstated fact stays absent rather than becoming a default, so a consumer can report what the session never said; a lane that needs a concrete value substitutes its own. resolve_precision_and_quant moves here from request_handlers, because it answers what the runtime is serving at rather than how one lane tunes. Its three copies of the fp8 auto-resolution collapse into one helper; the priority order and every outcome are unchanged, and the tests that pin them move with it. The one behaviour this settles is that the phase's own fp8 probe and the GEMM lane now ask the same function they always meant to. No consumers yet: the lanes are switched over next.
lane_inputs holds the projections: pure functions from a context and the lane's own execution knobs to the wrapper payload. No SharedState, no environment, no disk. What a lane receives can now be read in one place, and tested without a session. Four facts were reachable all along -- Hyperloom held the value and forge-fuse had the flag -- but nothing carried them across, so the A/B compared fused against unfused at the CLI's defaults rather than at the operating point the session measured. --ab-isl, --ab-osl and --decode-batch now come from the workload; --framework-root is sent only when the operator configured a checkout, because forge-fuse auto-detects the installed package and an empty flag is the instruction to go and look. That last one needs two fields rather than one. "Every repository a rewrite could name" and "the checkout the operator named" are different questions: the handoff wants all of them, a tool that can auto-detect wants only an explicit one. campaign_repositories moves to kernel_evidence. It is discovery -- which repositories exist -- and it was sitting in the module that also borrows, locks and seals them, so asking the question dragged in the repo-lock machinery and, with it, POSIX fcntl. A lane that only wants to name its source trees should not have to import any of that. Its seven tests move with it and now run on any platform. _fusion_session_serve_args is left with the one thing that was never a context fact: vLLM's KV block size, which comes from the model config rather than from anything the session recorded.
_ck_blockscale_switch_eligible imports resolve_precision_and_quant from kernel_context, so a test that replaces the attribute on request_handlers replaces one nothing calls and the probe falls back to the real resolver. Only CI could see it: this module imports the knowledge-plane local store, which needs POSIX fcntl, so it is not collectable on a Windows checkout.
The handoff is the rewrite lane's projection and the only one whose reader is a language model. What it leaves out, the opportunity analyst has to guess. Four things it left out, each reachable all along: - GPU and quantization. task.json requires a normalized identity.gpu and forge-loop derives --gpu-target from it, so the analyst was inferring which accelerator it was tuning for. - The serving log and the trace shape manifest. Its prompt tells it to cross-check TraceLens against the serving logs and to derive shape cases from the serving state; both were resolved for the GEMM lane alone, so it could do neither. Precision now comes from the same resolver the GEMM lane uses, so the two stop disagreeing about what the runtime is serving: the handoff read a session field that can predate the running server. Sealed baselines stay an argument rather than a context field. Sealing makes a commit, so it is materialization; the projection prefers what the seal returned over what discovery found, because that object id is the base every rewrite diffs from. forge_handoff_dir goes with the default it served. The handoff has ridden inside its attempt directory since a second entry in one macro cycle stopped being allowed to overwrite the first one's evidence. The suite splits along the same line the code does. Only the round-trip through KernelForge's own reader needs the controller import -- and with it POSIX fcntl -- so it moves to its own module and the thirteen projection tests become runnable on any platform, where before the whole file was uncollectable off Linux.
The last lane to assemble its own facts. Precision, quantization, model path, tp, conc, gpu_type, the serving log, the untuned CSV and the trace shape manifest all come from the context now, so the three lanes finally answer the same questions the same way. Nine environment reads go with it -- MODEL_PATH, TP, CONC, GPU_TYPE and the rest were re-reading fields SharedState already carries, and a second source for a fact is how the lanes came to disagree in the first place. Materialization stays. The block-FP8 profile reuse, the TunableOp capture, the MoE CSV writer and the aiter re-keying all mutate something or boot a server, so they run in the lane and their results reach gemm_input as GemmShapeSources -- what survived, rather than what was discovered. Where the context leaves an unstated fact absent the lane applies its own default (tp=1, conc=64, gpu_type=mi300x), because the tuner requires concrete values. That choice is now visible in one place instead of being folded into nine `or` chains. --demand goes. It is a pass-through no code path fills, and now that the payload is generated by a projection rather than hand-assembled, nothing can: forge reconstructs demand from --kernel-signature-log, which is always resolved. A test pins that the port stays gone.
GEMM was written inline in the entry hook while fusion and the rewrite controller sat a level down in _finish_kernel_entry. One lane in two places bought three copies of the same scaffolding and one real defect. The scaffolding: three hand-written bus posts, three evidence rows, and three failure envelopes that did not agree -- GEMM said error_class, fusion added engine, the controller said reason and patch_count, so which key carried the reason depended on which lane failed. The defect: the code's own rule is that a skipped lane returns inside its own helper, so INFERENCE_OPTIMIZER_SKIP_GEMM_TUNING cannot reach the controller. GEMM broke it. Its gate returned from the entry hook, which meant duplicating the entire tail as a second exit -- two paths to keep in step, and the reason the gate for GEMM ended up in loop/dispatcher.py while fusion's sat here. Now _on_enter_kernel is the GEAK branch and a loop. _run_forge_lane owns the gate, the snapshot refresh, the evidence row, the failure envelope and the response; each lane owns what it means to run. The order is unchanged and is stated where it can be read: GEMM tunes the tables the later lanes measure against, and fusion changes the decode path the controller then traces. GEAK is untouched. It is not a lane -- it owns the whole phase and returns before any of this -- so it stays the top-level branch it was. Two lanes keep a narrow catch around their own handler call, because last_fusion and the GEMM attempt ledger are their idempotency records and a crash belongs in them. The lane-level catch is the backstop for a defect anywhere else, and both build the envelope with the same helper.
--iters, --warmup, --min-improvement-pct, --gpu-ids and --verbose survived from when the GEMM input JSON was hand-assembled and an operator could reach in. It is generated by gemm_input now, which has no such fields and no way to acquire them, so every one of these read a key that is never set. The run_optimization lifecycle label goes with them: the request kind was removed in #1408 and nothing can emit a step by that name any more. Also corrects the --shapes-manifest comment, which still described the gap it was added to close rather than what it does.
…context # Conflicts: # src/hyperloom/orchestrator/kernel/request_handlers.py # src/hyperloom/orchestrator/phases/kernel.py
Three tests still described the entry as it was before the lanes became a sequence, and only CI could see it: all three modules reach the knowledge-plane local store, which needs POSIX fcntl and is uncollectable on a Windows checkout. - The GEMM-skip regression test drove _run_forge_fusion, which is now _run_fusion_lane. Keeping it working matters more than the rename: it is the test that pins INFERENCE_OPTIMIZER_SKIP_GEMM_TUNING not reaching fusion, which is the defect the sequence was built to remove. - Two stubs returned None where a lane returns its result, so the sequence had nothing to report. A lane's run() answering with a result is the contract; the stubs now honour it rather than the caller tolerating None. - _handle_fusion_result no longer posts the response, so the assertion that it does moves to where the response is written. The pair that must agree -- the bus status and last_fusion after an unreadable envelope downgrades it -- is pinned in test_fusion_envelope_skew.
Three leftovers from the two deletions, all of them assertions that the thing still exists. - The gemm wrapper's option test still declared and asserted --iters, --warmup, --min-improvement-pct, --gpu-ids and --verbose. Its meta-guard is a subset check, so a stale entry there is invisible; the payload and the --verbose assertion are what failed. - Two lifecycle tests used run_optimization as their worked example of a step resolving to a label. The kind was removed in #1408 and its label with it, so they now use run_gemm_tuning, which is live. - The prelude fixture builds a Coordinator field by field and had no bus, which was enough while only the GEMM branch posted a response. Every lane reports its outcome now, including one whose own work was stubbed out, so the fixture supplies one.
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
2 participants
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.
Why
request_handlers.pyis five things at once: the request dispatch table, threelane drivers, evidence discovery, shape materialization, and the integrate
path. 6758 lines, 37 environment reads across 23 variables.
The visible cost is that the same fact reaches the three KERNEL lanes in three
shapes, and a fact nobody wired reaches none of them.
precision: the GEMM lane parsescurrent_best's server args, checksSGLANG_USE_AITER_FP8_PER_TOKENand reads the checkpoint's config; therewrite handoff takes
state.precision. One session, two answers.model_path: the GEMM lane resolves a repo id to a local snapshot and skipswhen it cannot; the handoff writes the logical string, so the analysis agent
can be handed something it cannot read
config.jsonfrom.--ab-isl/--ab-osl/--decode-batch/--framework-root: forge-fuseaccepts all four, nothing forwards them, so the fusion A/B compares at the
CLI's defaults rather than the operating point the session measured.
workload.mdcarries no GPU and no quantization, though the controller'stask contract requires a normalized
identity.gpuand forge-loop derives--gpu-targetfrom it.alone, while the opportunity agent's own prompt tells it to cross-check
TraceLens against serving logs and to derive shape cases from them.
Any one of those is a three-place edit today.
Plan
Three cuts, staged so each commit is reviewable on its own.
KernelContext— workload, serving surface,evidence index — collected once. Discovery only: every field is a read, so
building one can never cost a GPU. Materialization stays in the lane.
gate/plan/run/normalize, with the phaseentry reduced to a sequence over the three lanes in their current order
(GEMM tuning, fusion, rewrite controller). Today GEMM alone is written
inline in the entry hook while the other two sit a level down, which is
where the duplicated error envelopes, the three hand-written bus posts and
the two exits come from.
through the same call shape.
integrate_handler's internals are out ofscope for this PR.
GEAK is untouched throughout. It is not a lane — it owns the whole phase and
returns before any of this runs — so it stays a top-level branch rather than
being fitted to the lane protocol.
Commits so far
refactor(kernel): move evidence discovery out of request_handlers—kernel_evidence.py. Pure move, no behaviour change.Test plan
pytest src/hyperloom/orchestrator/kernel/tests/ src/hyperloom/inference_optimizer/tests/test_kernel_request_handlers_units.py: 481 passed, against 484 on the parent commit — the difference is three redundant server-log cases deleted deliberately, and the 5 failures / 2 collection errors are identical to the parent's (Windows path separators, symlinks, andfcntlblockingkernelforge.loop.editable_repo).ruff check .andruff format --check .clean on ruff 0.16.2, the version pinned inlint.yml.🤖 Generated with Claude Code