fix(kb): address one kernel's records by one address, however its source and its environment spell it - #1529
chennyiiis wants to merge 4 commits into
Conversation
40f7fde to
61a703b
Compare
|
Reviewed the alias tier end to end against the merge base ( 1. Two operators declared in the same source file collide on the alias tier (correctness, regression vs
|
Blocking
Two consequences, both verified against this branch with
Suggest propagating Checked, no issue found
|
61a703b to
0c2d261
Compare
|
@xiaofei-zheng thank you — issue 1 decided the design. The alias tier is gone; this PR now does something much smaller. Force-pushed What your repro turned out to be a sample of. I ran a read-only scan of the production store (
So the blast radius of signature-equality-as-an-alias-test is the majority structure of the store, not a corner case, and the benefit it was buying is one fork whose two pages hold the same 1.4439 champion — a wasted campaign budget, not a lost speedup. Issues 2 and 3 were real too; I'm not fixing them, because the code they're in is deleted. What the defect actually was. The one genuine rename is The third design, also rejected on the data. Before landing this I costed "derive Migration. Of 106 distinct names, 9 contain an unbroken 11+ letter run and 4 of those are camel-squashed ( All four CodeQL cyclic-import alerts were on the deleted imports and are gone. Suite on crsuse2-m2m-069: |
0c2d261 to
ae5c89c
Compare
…spells it A forge-loop record is addressed by the operator as the campaign declared it, and the campaign declares the entry point as the source it read spells it -- which is both ways across one tree: KdaPackedDecodeKernel in the header that declares the kernel, kda_packed_decode_kernel in the module that binds it. Operator normalization lowercased without splitting camel case, so the two spellings produced kdapackeddecodekernel and kda_packed_decode: different pages, and the second addresses one nothing ever wrote to. The read is exact, so the miss surfaced as no_prior_record, indistinguishable from an operator nobody has ever ported -- a validated 1.44x port stayed on disk, still applying cleanly, while the campaign that should have started from it re-derived the kernel from scratch. The task contract makes this the common case rather than a rare one. It asks for the camel-cased, namespace-qualified spelling -- aiter::fusedAddRmsNorm, not fused_add_rms_norm -- because upstream pull-request search splits that spelling into terms and a name normalized before it arrives has no boundaries left to split on. Every kernel whose Python binding is snake-cased therefore has both spellings within reach of the agent that reads it. Normalization now splits camel-case boundaries before lowercasing. An acronym run stays one word: splitting on every case change would cut MoE into mo_e and QKV into q_k_v, inventing a difference between spellings of one kernel instead of removing one. Names already written as words normalize to themselves, so the pages that move are the camel-cased spellings that were squashed -- and the reported one merges into the page it forked from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ae5c89c to
391aa91
Compare
The version dimension of a knowledge-base address was whatever string the campaign happened to hold. Three code paths answer "which framework version?" and they answer in three different words -- importlib.metadata reports the build the wheel was compiled as (0.24.0+rocm723), a campaign reads the package tag (v0.24.0) or the image it arrived in (v0.5.15.post1-rocm720-mi35x-20260724), and one that observed nothing says none, unspecified or unknown depending on which path produced it. A port does not depend on the machine that compiled its framework, nor on the tag convention of whoever wrote the version down, so each of those spellings opened its own page for one kernel and no campaign could read the others. Measured against the production store's 146 forge-loop pages: 11 pages merge, 9 of them provably one kernel by identical implementation_signature, and every one of those 9 held a different champion on each page -- sparse_attn_prefill_ragged at 13.73x on v0.24.0 and 8.23x on 0.24.0+rocm723, mxfp8_grouped_gemm 4.18x against 3.39x, rmsnorm 1.49x against 1.16x. Each campaign re-derived a port the other had already validated. canonical_framework_version resolves a version string to the release it names, and every word for not having observed one to a single word. It is applied at the three points that write the dimension and on the read side, so no stored page is rewritten: the fuzzy tier reaches legacy spellings as the release they name rather than as a neighbouring release, which also un-strands the 42% of pages that name no version -- 12 of them reach a donor that nothing could reach before. Ranking keeps the one thing it refused before: how far a known release sits from an unknown one is not a question the strings can answer, so that comparison stays rejected rather than being given an invented distance.
An unregistered backend does not reach forge-loop as itself. Campaign setup resolves --kernel-backend against the same registry the agent's prompt is built from, substitutes the fallback, warns on stderr and continues -- so the run builds with one technology while the task directory, the experience id and the published pointer all say another. That is two addresses for one run, which is the defect this module exists to prevent rather than one to create. The comment standing here said the opposite: that forge-loop answers an unregistered backend with no layer rather than an error. It does not, and the rule it justified let the substitution through silently. Every backend in the store is already a registered one, so the refusal takes nothing away from what campaigns write today; the name the agent must spell is the one its own prompt hands it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The declared framework reaches forge-loop as an override, and an override short-circuits the path inference the detector exists for -- so the dimension travels unexamined all the way to the page a result is filed on. Two things go wrong there. A draft spelling "this kernel belongs to no package" its own way names one page in its directory and its published pointer while the run, which resolves standalone and none to unknown, files its result on another; the store holds one unknown and no standalone, which is that gap seen from the far end. And a framework naming a package the source does not live in is never contradicted by anything downstream, because the override suppressed the one check that could. Publication now folds the override the way the run will, and refuses a framework that none of the task's own paths sit under. It refuses and never re-derives: three stored pages sit at sglang/aiter/ops/flydsl/..., under two packages at once, and which one owns the kernel is not a question the order of a tuple should answer -- a declared framework appearing anywhere in the paths stands. A path under no known package witnesses nothing and is left alone, which is what keeps the four pages stored under torch addressable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The defect
A forge-loop record is addressed by the operator as the campaign declared it, and the campaign declares the entry point as the source it read spells it. One tree spells a kernel both ways:
KdaPackedDecodeKernelkdapackeddecodekernelkda_packed_decode_kernelkda_packed_decodenormalize_operator_namelowercased, replaced., collapsed separators and stripped a trailing_kernel— but never split camel case. So the two spellings addressed two pages, and the second addresses one nothing ever wrote to.The read is exact, so the miss surfaced as
no_prior_record: indistinguishable from an operator nobody has ever ported. A validated 1.44x port stayed on disk, still applying cleanly, while the campaign that should have started from it re-derived the kernel from scratch.Why this is the common case, not a rare one
The task contract asks for the camel-cased, namespace-qualified spelling:
That instruction is right about search and was silently wrong about addressing. Every kernel whose Python binding is snake-cased has both spellings within reach of the agent that reads it.
The change
normalize_operator_namesplits camel-case boundaries before lowercasing.An acronym run stays one word. Splitting on every case change would cut
MoEintomo_eandQKVintoq_k_v— inventing a difference between spellings of one kernel instead of removing one. The tail of a capitalized run only starts a new word when a lowercase letter follows it, and a one-character piece is given back to the word it was cut from only when that word is itself a fragment of at most two letters — which is what an acronym written with a lowercase letter inside looks like once the boundaries have cut it. After a whole word a capital is a word of its own.No other identity dimension moves, and the write and read sides share this function, so they cannot disagree.
Validated against every real kernel name reachable from here
The hand-picked examples above are not evidence that the rule is right, so it was property-tested over three corpora rather than over examples: the 107 distinct page names in the production store, the 201
implementation_symbolsrecorded across its 144 pages, and the 632 kernel symbols declared in this tree (@triton.jitdefs,__global__entry points,*_kerneldefs, C++ camel entry points) — 926 distinct names. For every snake-cased name in that union, both its Pascal and its camel spelling were generated and required to normalize to the same page.This found a real defect in the first version of the rule. Giving a trailing capital back unconditionally kept
MoEwhole but also welded the dimension letter these kernels end in onto the word before it:ChunkFwdKernelObecamechunk_fwd_kernelowhile the source declaring it spells itchunk_fwd_kernel_o. 82 of 769 kernel names, 230 of 1538 generated cases. The two-letter fragment bound above is the fix; the two classes are separable because an acronym cut in half leaves a fragment and a word does not._kernelpairs belowThe 29 disagreements are all one ambiguity and it is not decidable: a digit beside a letter.
mxfp4_moe_2stagesays2stageis one word,Mxfp4Moe2Stagesays it is two, andgemm_a8w8_blockscalesaysa8w8is one — a rule that split digits from letters would re-address that page togemm_a_8_w_8, which is worse than the gap. 28 of the 29 are of this shape; the last,recompute_w_u_fwd_kernel, spells two adjacent single letters that Pascal writesWU, indistinguishable from an acronym. 9 live page names fall in this set, and each costs a page only if a later campaign supplies the camel spelling of one.test_a_digit_run_keeps_whatever_boundary_the_spelling_gave_itpins the gap so a change in it cannot pass unnoticed.Measured against the production store
Read-only scan of
producer=forge-loop: 144 pages, 106 distinct names. Every page's champion record was fetched and compared; nothing was written.No stored page is rewritten, and there is no migration. What changes is the page a later campaign addresses. Two of the 106 names land elsewhere:
kdapackeddecodekernelkda_packed_decode— already existstopkgatingsoftmaxtopk_gating_softmax— nothing there yetThe merge is the fork this PR reports, and the store proves the two pages are the same kernel rather than two that happen to share a name:
kda_packed_decodekdapackeddecodekernelimplementation_signature527b02951b281dac…527b02951b281dac…implementation_symbols["kda_packed_decode_kernel"]["kda_packed_decode_kernel"]source_pathssglang/kernels/…/kda_packed_decode.{cuh,py}Two campaigns, one kernel, the same result derived twice.
The orphan is conditional rather than certain.
topkgatingsoftmax's record carries the source symboltopkGatingSoftmax, which the old rule squashes to the current page name and the new rule splits — so the page moves only if a later campaign again names the operator from that symbol. Its history is one record.Eight further names read like squashed camel (
hipbsolgemm,hipgraphlaunch_*,gemm_a8w8_*_bpreshuffle,rocm_unquantized_gemm) but no symbol in their records shows a camel spelling, and the new rule is the identity on a lowercase input. They move only if a campaign supplies the camel spelling, and no page exists at the name they would move to, so nothing merges and nothing is overwritten.No two stored pages can collide. Every stored name is already lowercase and therefore its own normal form, so the new rule maps the 106 names injectively.
What the same scan ruled out
The scan found 18 groups where one
implementation_signaturespans several names. Only thekdapair is a rename. The other 17 are distinct operators that share a source file and therefore share a file-level signature — six different operators invllm/_aiter_ops.pyalone,flydsl_moe_stage1besidestage2,ck_moe_stage1besidestage2,mxfp4_moe_2stage_t16/t32/t64/t128. That ruled out the two designs this PR previously carried and one considered after it:match_mode='exact'and then file its result on the sibling's page. @xiaofei-zheng raised exactly this; the store says it is the majority structure, not a corner case.unified_attention_with_outputtoapply_softcap, collapsesmxfp4_moe_2stage_t16/t32/t64/t128into one name andcustom_all_reduce_tp2/tp4into another.operator_namenames the right thing. Only its normalization was lossy.The same defect in the version dimension
The same scan found the address splitting a second way, for the same reason: a dimension was whatever string the campaign happened to hold rather than the fact it names.
Three code paths answer "which framework version?" and they answer in three different words.
framework_version()viaimportlib.metadata0.24.0+rocm723,0.5.15.post1.dev20260724+g3d91a569cev0.24.0,v0.5.15.post1-rocm720-mi35x-20260724none,unspecified,unknown,""A port is written against a release. It does not depend on the machine that compiled the framework, nor on the tag convention of whoever wrote the version down, nor on which of three code paths failed to find a version. Each of those spellings opened its own page.
What it cost, measured
Read-only scan of the same 146
producer=forge-looppages, replaying the canonicalizer this PR ships and fetching each page's champion record:146 pages → 135. Eleven merge, nine of them provably one kernel by identical
implementation_signature. Every one of those nine held a different champion on each page:sparse_attn_prefill_raggedv0.24.0/0.24.0+rocm723mxfp8_grouped_gemm0.5.15.post1.dev20260724+g3d91a569ce/v0.5.15.post1-rocm720-mi35x-20260724mhc_fused_post_pre0.24.0+rocm723/v0.24.0unified_attention_with_outputunspecified/unknownpaged_attention_rocmunspecified/unknownrmsnormunknown/unspecifiedfused_moe0.24.0+rocm723/v0.24.0aiter_mxfp4_moe_2stageunknown/unspecifiedmoe_flydsl_stage2unspecified/unknownEach row is one kernel ported twice, by two campaigns that could not see each other. The remaining two groups share an operator but not a signature, and both elect the faster of the two:
dynamic_per_tensor_quant10.4489x over 2.8170x,unified_attention_with_output@0.24.011.5837x over 2.5712x.The change
canonical_framework_versionresolves a version string to the release it names, and every word for not having observed one to a single word.Parsing is PEP 440 (
packaging.version.Version, already a hard dependency ofwarmstart_identity). An image tag joins the build to the release with the same character a pre-release uses, so it is only distinguishable by failing to parse whole — hence the one retry on the first-. A string naming no release is returned exactly as written, so the read side and the write side, which canonicalize independently, cannot turn it into some other page's name.It is applied at the three points that write the dimension and once on the read side:
rewrite_by_flydsl/identity.py: framework_version()NO_FRAMEWORK_VERSION = "none"andUNKNOWN_VERSION = "unspecified", both deletedkernel_identity.py: KernelRecipeIdentity.from_mappingtask.jsontask_publisher.py: _normalize_agent_task_payloadkernel_nameit already re-deriveswarmstart_identity.py: rank_fallback_identitiesNo stored page is rewritten and there is no migration. The fuzzy tier reads a legacy spelling as the release it names rather than as a neighbouring release, so the histories above stay reachable from the canonical address.
This also un-strands the pages that never had a version. 55 of 146 name no release — 42% — and the fuzzy tier used to end before it started for all of them, because
unknownfails to parse. Under one word for absence, 12 of them now reach a donor that nothing could reach before.What the ranking still refuses is unchanged: how far a known release sits from an unknown one is not a question the strings can answer, so that comparison stays rejected rather than being given an invented distance.
test_fuzzy_ranking_rejects_unknown_or_unparseable_dimensionspasses untouched.Where the six dimensions stand after this
producer== forge-loopcheck intask.pykernel_namenormalize_operator_name, re-derived host-side fromoperator_nameframeworkframework_versioncanonical_framework_versionbackendKERNEL_BACKENDS, the set campaign setup resolves againstgpumi355x)The same defect in the backend dimension
identity.backendis one of the five dimensions the agent types intotask.json, and it becomes--kernel-backendon the forge-loop command line.campaign_setup.resolve_kernel_backend_overridethen resolves it againstKERNEL_BACKENDSand, for a name that is not there, substitutes_FALLBACK_KERNEL_BACKEND = "flydsl", warns on stderr, and continues.So a draft saying
tilelangproduces a run that builds with flydsl and files its result on the flydsl page, while the task directory, the--experience-idand the published pointer all saytilelang. One run, two addresses — the shape this PR exists to remove.The comment standing at that spot in
task.pyjustified not checking, and stated the reason as fact:It does not. It answers with a different layer, silently. The rule that comment defended is what let the substitution through.
This commit checks membership in
KERNEL_BACKENDS— the same frozen setcampaign_setupresolves against, and the same oneopportunity_agent.py:418already injects into the agent's prompt. The name the agent has to spell is the one it was handed.assembly,ck,flydsl,triton,gluon,aiter,hip,hipblaslt,fusiontriton81,hip32,flydsl19,ck11,aiter3The refusal costs nothing that campaigns write today. It only closes the path by which a name the agent invents becomes a page nothing writes to.
The same defect in the framework dimension
detect_framework(kernel_path, framework_override)infers the owning package from the path — but a non-empty override short-circuits the inference entirely. Since the agent'sidentity.frameworktravels to forge-loop as exactly that override, the dimension reaches the knowledge-base page having never been examined by anything. Two consequences:1. A kernel that belongs to no package is addressed two ways.
_NO_FRAMEWORK_SENTINELS = {"standalone", "none", "unknown"}all fold tounknowninside the run, but the draft keeps its own spelling in its directory name and its published pointer. The store is the same gap seen from the far end:standaloneunknownEvery campaign that declared
standalonefiled its result underunknownand pointed atstandalone.2. A framework naming a package the source does not live in is never contradicted. The override suppressed the one check that could have noticed.
This commit folds the override at publication the way the run will, and refuses a framework that none of the task's own paths sit under. The paths were just verified to exist at the base commit, which makes them the only witness available at that point.
Why it refuses rather than re-derives
I first wrote this to derive
frameworkfrom the paths and overwrite whatever the agent said. Scanning the store killed that design:All 3 disagreements are the same path:
sglang/aiter/ops/flydsl/kernels/mixed_moe_gemm_2stage.py— under both packages at once.detect_frameworkanswersaiteronly because_FRAMEWORKS = ("aiter", "sglang", "vllm")starts there. Re-deriving would have moved those three pages on the strength of a tuple's ordering.So the check refuses a contradiction and never re-derives. A declared framework appearing anywhere in the paths stands. A path under no known package witnesses nothing and is left alone — which is what keeps the 4 pages stored under
torch(a value_FRAMEWORKSdoes not know) addressable.Not covered
gpuis the one dimension of the six with an alias the write side does not fold.warmstart_identity._GPU_ISAalready knowsmi355xandamd_instinct_mi355xare one card, andmi300x/amd_instinct_mi300xlikewise — but only on the read side, for ranking. Nothing folds them when the address is written. All 146 stored pages saymi355x, so there is no split to repair today; the fix shape is acanonical_gpu()fed by that same alias table, and it is a separate concern from the three here. It is the only one of the six this branch leaves unpinned._kernel—mxfp8_quant_kernel,mxfp8_grouped_gemm_kernel,fwd_kernel,llgemm1_kernel,dynamic_per_group_scaled_quant_kernel,recompute_w_u_fwd_kernel,tp2_fused_ar_rms_kernel— whichnormalize_operator_namehas stripped since before this PR, so no campaign can address them today either.mxfp8_quant_kernelsits besidemxfp8_quantandmxfp8_grouped_gemm_kernelbesidemxfp8_grouped_gemm: the same two-half-histories shape, but it implies a write path that bypasses the normalizer, which is a different defect and not traced here.match_modemeans "same files", not "same kernel". Given the file-level signature,implementation_matchis true for siblings in one file. It labels the reference handed to the model and gates nothing — warm start is decided by whether the patch applies cleanly and benchmarks faster — so this is a naming problem, not a behavioural one, and fixing it needs an operator selector in the signature payload.implementation_identitywas recorded (74 of 144) carry no symbols; nothing here reaches them, and nothing needs to.operator_nameis not required to appear intarget_functions), separate from spelling.Test
test_the_same_kernel_spelled_either_way_is_one_operator— the reported pair.test_namespaced_camel_case_matches_its_snake_case_spelling— the spelling the contract asks for costs no page.test_an_acronym_run_is_one_word—MoE,KVCache,QKV,HGEMV_WFPerRow.test_a_capital_after_a_whole_word_is_its_own_word— the counterpart, and the regression for the defect the validation above found.test_a_digit_run_keeps_whatever_boundary_the_spelling_gave_it— the known gap, pinned.test_names_already_written_as_words_are_left_alone— real page names from the store normalize to themselves.test_distinct_operators_sharing_a_prefix_stay_distinct—stage1≠stage2,rmsNorm≠addRmsNorm.test_either_spelling_of_one_operator_reaches_the_same_record— end to end throughresolve_loop_identity: written under one spelling, found under all three.Version dimension:
test_the_build_a_release_was_compiled_as_is_not_part_of_the_release—0.24.0+rocm723, and a dev build with three suffixes.test_a_release_reads_the_same_however_it_was_written_down— the tag spelling and the image spelling reach the release.test_every_word_for_not_knowing_the_version_is_the_same_word— all four absences.test_distinct_releases_stay_distinct—.post1,rc1and a patch bump are not folded away.test_a_version_naming_no_release_is_left_exactly_as_written— idempotence, which the independently-canonicalizing read and write sides depend on.test_two_spellings_of_one_release_rank_as_that_release— the fuzzy tier reads a legacy page as the release it names.test_runs_that_both_observed_no_version_can_reach_each_other— the 42%, with the known-vs-unknown comparison still refused.Backend dimension:
test_an_unregistered_backend_is_refused_rather_than_silently_substituted— replacestest_an_unregistered_backend_is_accepted, which pinned the old behaviour and repeated the false premise in its docstring.test_every_backend_the_agent_is_offered_is_one_it_may_declare— every name inKERNEL_BACKENDSsurvives the check, so the refusal cannot cost an operator a name its prompt offered.Framework dimension:
test_a_kernel_belonging_to_no_package_is_published_under_one_word_for_that—standaloneis published asunknown, which is where the run files its result.test_a_framework_alias_is_published_as_the_package_it_names—aiter_meta→aiter.test_a_framework_none_of_the_paths_sit_under_is_refused—vllmdeclared over asglang/path leaves a rejection the agent can read.test_a_path_under_two_packages_keeps_whichever_the_task_declared— the realsglang/aiter/ops/flydsl/...case: declaredsglangstands.test_a_path_under_no_known_package_witnesses_nothing—torch, which no path names, is left alone.The first commit was verified on crsuse2-m2m-069 (8x MI355X):
4351 passed; the 8 failures in that run are*_gpu.pytests that fail identically on an unpatched tree (No module named 'torch'in the interpreter). That run predates the two-letter-fragment amendment and all three later commits, and the box has been unreachable since — it owes a re-run before merge.Locally, the two commits added here land entirely inside the controller package, which Windows could not collect at all before:
fcntlis absent, so itsconftestfails to import and every test in it is unreachable. For this round I stubbedfcntl.flockand the directoryfsyncout of process — neither stub is in the repository, and neither proves the lock contention or the durability the real calls exist for — which made the package runnable for the first time. Run on this branch and on a detached worktree at the parent commit:bd77313dfThose 27 are the platform's:
os.openon a directory andos.fsyncon a read-only descriptor are POSIX abilities Windows lacks, symlink creation needs a privilege this account does not hold, and the fourtest_controller_full_pathcases fail on both trees before reaching any assertion of mine.That last point is the one gap worth naming:
test_controller_full_pathis where the agent's draft and the published address are both visible, and it hardcodedstandalonein the operator id. This branch updates that constant tounknown, and the local run cannot confirm the update — the test fails for its own reasons on both trees. What does confirm it istest_a_kernel_belonging_to_no_package_is_published_under_one_word_for_that, which passes and drives publication over the identical six-tuple.Outside the controller package, the modules these commits import from were diffed the same way —
test_assembly_backend(the only test file outside the package that imports the controller),test_implementation_identity,test_warmstart_identity,test_experience_sinkandtest_experience_sink_helpers: 106 passed, 2 failed, byte-identical on both trees. The earlier version commit was diffed over the whole suite (1886 → 1893 passed, 223 failed / 116 errors unchanged, identical failing ids).The whole suite was then diffed the same way. The stubs make it a much larger run than the figures above: the 69 files
fcntlused to block are collected, so the total goes from 1893 to 4016.bd77313dfThe 292 are overwhelmingly the platform's — directory
os.open,os.fsyncon a read-only descriptor, symlink privilege — and being identical on both trees is what says so. None of this substitutes for the devbox re-run, which is what actually decides these commits.