Add xDiT kernel source mapping support - #1154
Open
kyle-hoffmeyer wants to merge 764 commits into
Open
Conversation
The lease guard was a two-item parametrize list while the description called it an invariant with no exceptions. It now reads each cap out of the executor's own signature, so a moved default is caught, and states what it does not cover: explore declares a 7200s lease against a runtime-derived ceiling of 14400s, so the same contradiction is reachable there. Its cap only exists at runtime, so there is no static number to assert against; recorded rather than implied. Co-authored-by: Cursor <cursoragent@cursor.com>
…r-normalization refactor(llm): treat DeepSeek as a dual-protocol gateway, not a provider
The benchmark backend defaults to Magpie, which cannot run an operator-supplied script, and nothing on the way in said so: `_apply_operator_supplied_paths` validated the two directories and let the launch proceed, so an unset or mistyped `HYPERLOOM_BENCHMARK_BACKEND` simply took the wrong executor and failed later, somewhere harder to attribute. The how-to promised this was caught -- "omitting it fails at launch with `Unsupported framework`" -- but that string does not exist anywhere in the product; the only occurrence in the tree is a test comment. So the documented guard was fictional as well as absent. `custom` now refuses any value but bypass, alongside the existing path checks and reported with them. The normalisation matches install.sh's so the two gates cannot disagree about a value like " Bypass ". The gate is scoped to custom: shipped frameworks keep whatever backend they launch with today, and a test pins that. Co-authored-by: Cursor <cursoragent@cursor.com>
Moving the budget guards to cumulative accounting left their phase_history evidence behind: all seven budget exits still recorded `phase_elapsed_seconds`, which is the current entry only. On a re-entered phase that reads as a contradiction — a row claiming the allotment is spent while showing a few minutes elapsed — and phase_history is exactly what a stalled run is reconstructed from afterwards. Each of those exits now carries `cumulative_elapsed_seconds`, the number the decision was actually made on, next to the per-entry figure renamed to `entry_elapsed_seconds`. Nothing in the product reads the old key; the same omission on the prompt status line was fixed when the guards moved. Co-authored-by: Cursor <cursoragent@cursor.com>
This was the one thing on the branch that could stop a session, and its reason had already evaporated. The variable was dangerous because only the reader honoured it, so setting it split the KB in two without saying so. Now that the reader and kb_writeback both resolve through mutable_kb_root, it is inert: the KB lands in the same correct place whether or not it is exported. Refusing to start guarded nothing and put every run on a deployment still carrying it at the mercy of a file someone forgot about — on the strength of one dev box having no such variable, which is exactly the single-machine evidence this description already flags. It now warns, naming both the replacement and the root the KB actually resolved to, so an operator who did mean to move it can see where it went. The guarantee is made structural rather than left to inspection: prepare_kb_environment() as a whole cannot raise, so a start-up step added later inherits that instead of having to remember it, and test_start_up_never_raises fails if the property is lost. KBConfigurationError and the fa CLI's handling of it go with the behaviour. The audit note never asked for this. Its remedy for the split was to give the ledger a single authority; withdrawing the variable and failing on it was a design decision layered on top, and it is the layer that turned a fix into a blocker. Co-authored-by: Cursor <cursoragent@cursor.com>
Raising it to match its bench cap was correct in itself but out of scope: the action is not part of the rename audit, it is used by EXPLORE, framework authoring and the enablement build probe, and the change rested on reading the code rather than on any measurement of how long it actually runs. The contradiction is real and pre-existing, so it is recorded where the next person will look — a comment on the declaration and the guard's own docstring, alongside explore, which has the same problem in a form no static number can fix. The guard now covers framework_agent alone, which is the action the audit was about and the only one whose upper bound is a static number. Co-authored-by: Cursor <cursoragent@cursor.com>
…enchmark Add an operator-supplied workload path and a source-rewrite specialist for iterative pipelines
…nAI-only KernelForge registers both a `claude` and a `codex` agent provider, and its `Config.agent_backend` defaults to `auto`, which resolves to `claude`. Hyperloom built the `forge-loop` argv with none of KernelForge's provider options (`--agent-backend`, `--model`, `--agent-fallback-provider`), so an OpenAI-only deployment -- no Anthropic credential, no Claude CLI login -- sent every forge attempt to a provider that cannot authenticate. Observed in a full sglang/Qwen3-8B session: the attempt died in 13.5s with "prep agent never edited the driver in 3 attempt(s)", surfaced as `task_preparation_failed`, and the whole KERNEL phase produced nothing while consuming 118% of its budget. Three layers had to change, because fixing only the first is silently undone by the other two: * Select the provider. `_openai_only_provider()` mirrors the existing `tracelens_skill_runner._should_use_openai_tool_runner()` predicate, and pins `--agent-backend codex` plus the session's `CODEX_MODEL`. * Refuse the silent fallback. KernelForge's `agent_fallback_provider` defaults to `claude`, so a missing Codex SDK quietly produced a ClaudeBackend whose availability probe passes (it checks the binary, not the auth) and only failed at the first real turn. Pass `--agent-fallback-provider none` so it fails loudly at dispatch instead. * Install the SDK. install.sh installed kernel_agents with no extras, leaving `openai_codex` absent. It now installs `[claude,codex]`, and the "already importable" guard checks the codex SDK too -- otherwise a pod that already has kernel_agents skips the install and the extras never land. `_apply_fellow_env` keeps its claude CLI discovery, ANTHROPIC_BASE_URL rewrite and `~/.claude/config.json` key seeding behind the claude-fellow branch, so the OpenAI-only child no longer advertises a Claude CLI it cannot authenticate. The Anthropic-side ordering is unchanged. Also fixes a mislabel this work surfaced: tracelens_analysis hardcoded `orchestrator_mode = "claude_agent_sdk"` at the call site, so an OpenAI-only run whose analysis actually ran on the codex tool runner still reported the Claude SDK. TraceLensSkillRunResult now carries a required `runner` field that each runner declares, and the caller reports what actually ran. Stale docstrings that described the provider-neutral prompt builder and dispatcher as Claude-only are corrected. Verified on MI355X against a live OpenAI-only gateway: forge-loop with `agent_backend=codex, agent_model=gpt-5.5` optimized the Triton softmax example from 0.0173ms to 0.01578ms (1.096x) over 4 iterations, validation ALL PASSED at SNR 103.46dB. Co-authored-by: Cursor <cursoragent@cursor.com>
…ntent Six findings from reviewing this branch's own changes. The migration wrote a provenance marker into the partition it had just filled. `get_domain_files` returns every entry under a domain unfiltered and hands them to the KB readers, and `framework_optimization` is a served domain, so the marker came back out as a KB file. It was redundant anyway: being once-only comes from refusing a populated destination, not from a marker, and the provenance it carried is already in the log line. Dropped. `copytree` followed symlinks, so a link in the legacy partition arrived as a copy of whatever it pointed at — possibly from outside the workspace — in a directory the reader serves as its own. Copies links as links now. `kb_writeback` re-spelled `"framework_optimization"` while `kb.py` defines the constant for it: one name in two places, in a line this branch touched, which is the shape the whole branch is about. Both sides go through `framework_optimization_root()`. The v5 stack normalization edited entries in place, and `filtered` is a shallow copy, so it rewrote the caller's `raw` as well. `from_dict` does not own that mapping; the list is rebuilt instead. The empty-candidate-key behaviour change shipped with a log line and no test — the one behaviour change on this branch not pinned by a guard. Writing that test corrected the claim it was pinning: `current_best` and `cumulative_gain` are set unconditionally, and only the `optimization_stack` append sits behind the guard that the bare key now fails. So the win still counts; what it loses is a step anything could reconcile, dedupe or replay. Review described this as the gain no longer reaching `current_best`, and this description repeated that without checking. Both are corrected, and the log line now says which half is affected. Finally, the lease guard read `parameters["variant_timeout_sec"]` directly, which raises KeyError rather than failing with a reason if the executor ever stops taking it. Co-authored-by: Cursor <cursoragent@cursor.com>
One conflict, in `_collect_framework_agent_candidate_priors`: main added the `kept_inert` outcome to the terminal set this helper filters on, while this branch rewrote the same body to drop a try/except that was masking an AttributeError and to stop sending the Critic an empty `rationale` key. Resolved by keeping this branch's shape and taking main's new status: `kept_inert` is a real terminal verdict (`integrate_patch` emits it and writeback records it), so priors that omitted it would under-report what the Critic has already seen. `critic_denied` stays spelled as FRAMEWORK_CRITIC_DENIED_STATUS rather than main's literal, which is the same value through the constant that already exists for it. Everything else merged cleanly. Verified the branch's own changes survived: the empty-lane refusal and registry-sourced TTL in phases/framework.py, the v5 stack normalisation in shared_state.py, and the empty-candidate-key warning in writeback.py. Co-authored-by: Cursor <cursoragent@cursor.com>
Every call site built its own AsyncOpenAI, so provider credentials and client construction were spread across eight modules and provider selection was not a single decision. llm_config was already the config chokepoint; it now owns client ownership too. Add get_openai_client / get_async_openai_client as the sanctioned constructors: both resolve credentials through the existing resolve_openai_client_config, lazy-import the SDK so importing llm_config stays cheap, raise LLMConfigError with a clear install hint when the SDK is absent, and accept a per-caller transport timeout. Clients are deliberately not cached: an AsyncOpenAI's httpx pool binds to the event loop that first uses it and preflight rewrites provider env mid-process, so the owning object's lifetime is the only safe scope. build_http_timeout centralizes the connect/read httpx.Timeout tuning the critic backend open-coded, keeping its documented degradation to SDK defaults when httpx is unavailable. A timeout httpx itself rejects now raises instead of silently disabling the timeout. achat_completion flattens a non-streaming reply into text + finish_reason + usage, which is what the codex and critic paths both need. Migrate CodexBackend, CriticAgentBackend and ProposalScorer onto these entry points; none of them imports openai any more, and a static guard test keeps it that way. ProposalScorer's hand-rolled stream reader is replaced by the existing astream_chat_completion_text. A response carrying no choices now surfaces as a role-tagged LLMCallFailed (with the critic's failure trace row) rather than a bare IndexError, since that is exactly the "model request produced nothing usable" case LLMCallFailed documents.
…ontract Ad-hoc provider clients spread LLM credential policy across the tree, so any gateway/auth fix has to be applied in N places and is easy to miss. Move the last three sites that already used llm_config's streaming helpers onto its client-construction contract, and add an architecture guard so the property survives future changes. * framework/audit.py and orchestrator/phases/framework.py now ask llm_config.get_openai_client / get_async_openai_client for their client. Both keep their caller-visible degrade contract: audit appends its existing "llm refine skipped" risk and returns the static verdict, and the optional FRAMEWORK ranker stays disabled (now with a debug log instead of a silent swallow) when the client cannot be built. * cli/backends.py keeps _deepseek_openai_client_factory, because the DeepSeek default base URL and its /v1 normalization are caller policy that needs a documented home, but delegates construction to llm_config. It passes a minimal env mapping so the OPENAI_API_KEY / LLM_GATEWAY_KEY / OPENAI_CUSTOM_HEADERS fallbacks cannot route a foreign credential to the DeepSeek endpoint, and a missing key now fails loudly instead of yielding a client that 401s later. * common/tests/test_llm_client_architecture.py parses every first-party source and fails on provider SDK imports/construction, bare .chat.completions.create / .responses.create calls, and hand-rolled Anthropic Messages / OpenAI chat-completions HTTP posts outside llm_config and codex_session. Violations still pending migration are pinned as (path, rule) -> count and compared for exact equality, so the list fails on new debt and on stale pins alike and can only shrink. The detector and the ratchet both carry self-tests, since a guard that detects nothing passes vacuously. Tests for the migrated sites stub llm_config's client contract with raising=False, so they hold whether or not the contract has landed in a given checkout and no longer need the openai SDK installed.
The previous commit left one bare LLM call behind: CodexBackend's web-search path still reached self._client.responses.create() itself, so the Responses API had no sanctioned owner even though chat.completions did. Add aresponse() returning ResponsesResult, mirroring achat_completion / ChatCompletionResult: it issues the call, walks the output array for output_text blocks and url_citation annotations, and hands back exactly what the call site consumes. Transport errors propagate so the caller keeps its own role-tagged LLMCallFailed, and latency stays the caller's to measure. Token counters are flattened rather than passed through as raw usage, because the Responses API fixes their names while chat-completions callers still have to map provider-specific spellings. Migrate CodexBackend._run_responses onto it inside the same asyncio.wait_for and the same error translation. The local _field / _extract_responses_output helpers move into llm_config with the call they served, so the backend no longer carries Responses-API shape knowledge; their unit tests move to test_llm_config.py alongside the new entry point. Extend the static guard from "imports no openai" to "makes no bare chat.completions.create or responses.create call", which is the invariant that actually matters. Verified failing on the pre-fix tree at codex.py:344 and passing after.
… assert
Adding the codex extra rewrote two install.sh log lines from
"kernel_agents installed OK from <root>" to "kernel_agents + codex SDK installed
OK from <root>" (and the same for "already importable; skipping install"). That
moved the component name out of the slot the surrounding lines use --
"forge-gemm-tune installed OK from ${root}", "Magpie installed OK from ${spec}"
-- and broke the three assertions in test_install_kernel_agents_idempotent.py
that pin those prefixes, failing 6 CI test shards across py3.10 and py3.11.
Restore the conventional "<component> installed OK from <root>" prefix and carry
the extras as a trailing parenthetical, so the operator still sees that the codex
SDK landed while the asserted prefixes stay intact. No test changes needed.
Verified: the 3 previously failing assertions pass; 10142 tests pass across
inference_optimizer + kernel with `-n 16`. The 2 remaining failures
(test_bypass_report render assertion, test_aiter_build_e2e_real_rocm) reproduce
with this change stashed, so they are pre-existing and unrelated.
Co-authored-by: Cursor <cursoragent@cursor.com>
Hyperloom must never make bare LLM API calls: every interaction belongs inside an explicit agent runtime, either the Claude Agent SDK or the Codex SDK. The TraceLens skill runner violated that on its OpenAI-only path, hand-writing a 50-turn tool-calling loop against AsyncOpenAI().chat.completions.create with four bespoke tool schemas, a hand-rolled read-path sandbox (_OpenAIToolScope, _path_is_under, _resolve_tool_path, _validate_openai_command_argv), its own assistant-message re-serialization and its own usage accounting -- ~330 lines re-implementing what an agent SDK already provides. That machinery is replaced by openai_codex, which supplies the shell/file tools, an OS-level sandbox, turn management and usage reporting natively. New module src/hyperloom/common/codex_session.py owns the SDK plumbing shared by Hyperloom's Codex-side runners. It follows the patterns of KernelForge's kernel_agents.agent_backends.codex.CodexBackend but deliberately does not reuse the class: its _WorkspaceGuard.prepare() hard-requires a git worktree and enforces KernelForge's benchmark-file protection globs, neither of which fits a TraceLens output directory. The module keeps the reference gateway mapping's important property -- model_providers.<name>.env_key carries the NAME of the credential env var, never its value, so the secret never reaches the app-server argv. Gateway resolution delegates to llm_config.resolve_openai_client_config instead of re-reading os.environ, header names that cannot be expressed as a Codex config key are rejected loudly, CODEX_HOME is redirected to a per-run temp dir, and an overrunning turn is interrupted and then cancelled rather than leaked. The TraceLens session runs with ApprovalMode.deny_all so no approval prompt can stall an unattended run, cwd at the TraceLens root (matching the Claude path, so the skill's command-prefix cache and the TraceLens CLIs' relative paths resolve identically on both runners), and workspace_write scoped to that workspace plus the output dir. The result contract is unchanged: runner="codex", analysis.md stays the source of truth, and the agent_transcript.jsonl artifact is now derived from the SDK's typed thread items (commandExecution / fileChange / agentMessage) instead of chat-completion messages. This drops a silent truncation with it. The old loop capped every turn at max_completion_tokens=2000 while asking the model to write the full report. Measured with tiktoken o200k_base, TraceLens's own analysis_template.md is 6,243 tokens and 7,259 once escaped into the write_file call's arguments JSON -- 3.6x over the cap, before any reasoning tokens, which gpt-5.6-sol bills against the same budget. Truncation landed inside that arguments JSON, so json.loads failed and _execute_openai_tool returned a generic "invalid JSON arguments" tool result; the loop then burned another of its 50 turns with the model never told it had been cut off, and the run ended with "did not write analysis.md". The Codex SDK exposes no per-turn output cap at all (verified: no max_output_tokens / max_tokens anywhere in openai_codex), so the new path removes the cap rather than re-guessing it, and normalized usage reports reasoning_output_tokens so the tokens the old cap silently ate are visible in the transcript. budget_minutes stops being decorative: the Codex path spends it as the turn's wall-clock timeout, floored at 60s to match the other TraceLens subprocess timeouts. _should_use_openai_tool_runner keeps its name even though the OpenAI tool runner is gone, because forge_submit._openai_only_provider documents itself as mirroring that symbol and forge_submit.py is outside this change's scope; its docstring now states what the predicate actually selects. openai-codex is declared in the llm extra: with the OpenAI side routed through an agent runtime, an OpenAI-only deployment cannot run TraceLens without it. test_codex_session.py covers the gateway overrides (including that the key value never appears in them), sandbox/approval selection, CODEX_HOME isolation, timeout-then-interrupt and interrupt-then-cancel, usage normalization, the missing-SDK error, and guards the installed SDK's API surface so an upgrade fails in CI rather than mid-session. 100% line coverage on the new module. Full suite (pytest -n 32): 11,638 passed with the same pre-existing failures as the base commit. Co-authored-by: Cursor <cursoragent@cursor.com>
Store GBrain recipe argv without shell wrappers and repair existing quoted JSON so warm replay can launch vLLM configurations reliably. Co-authored-by: Cursor <cursoragent@cursor.com>
…-audit-fixes fix(framework): close the silent failures left by the framework_agent rename
…e one _ensure_python_sdks probe-installs the SDKs that are imported at runtime, but its candidate list carried only claude_agent_sdk, openai and httpx. Now that the TraceLens skill runner executes through the Codex Agent SDK and the forge fellow dispatches to the codex provider, an OpenAI-only deployment that reached a pod through preflight rather than install.sh had no runtime to execute either one, and failed at first use with CodexSessionUnavailableError. Add openai_codex to the probe-then-install list, matching the openai-codex>=0.144 floor already declared in the llm extra. Both runtimes now ship by default, which is what the three supported credential shapes require: Anthropic-only, OpenAI-only, or both. The two existing tests pinned the list length at three, so they now pin four, and a new test covers the codex install path specifically. Co-authored-by: Cursor <cursoragent@cursor.com>
The Anthropic Messages POST was hand-rolled in three separate copies, and the `anthropic-version` header was hardcoded in four places -- three of them inside AnthropicRcaEngine alone. `common/llm.py` contributed a fourth hand-rolled provider path (Anthropic Messages *and* OpenAI chat completions) on top of that. Retire `common/llm.py` and move its only importer plus the robustness RCA engines onto the `common/llm_config.py` contract, so provider credentials, client construction, gateway custom headers and the single `anthropic-version` default have exactly one owner. * `common/llm.py` is deleted outright rather than emptied: both its `/v1/messages` and its `/chat/completions` POSTs go away, and so does its module docstring asserting that role backends "continue to use the openai / claude_agent_sdk packages directly" -- the opposite of where the codebase is heading. * `breakdown/reporters/llm_client.py`, its only importer anywhere in the repo, now takes an already-built client: `build_client_from_env` obtains one from `get_openai_client` / `get_anthropic_client` and the adapters send through `stream_chat_completion_text` / `anthropic_messages`. It no longer resolves credentials itself, which also drops the vestigial `CLAUDE_API_KEY` fallback that appeared nowhere else in the tree; `ANTHROPIC_API_KEY` / `ANTHROPIC_AUTH_TOKEN` resolution is the contract's job. * `AnthropicRcaEngine` and `LlmRcaEngine` drop their `httpx.AsyncClient` copies for `get_async_anthropic_client` / `get_async_openai_client` and `aanthropic_messages` / `achat_completion`. Header patching on injected clients goes with them: provider isolation is structural now that each engine names exactly one factory. The client is built on first call instead of in `__post_init__`, so its connection pool binds to the event loop that issues the request rather than whichever loop happened to build the engine. Throttling and the `drain_usage` token ledger are deliberately untouched; only the transport underneath them changed. OpenAI usage is now read off the SDK object's attributes instead of a JSON mapping, matching how codex.py and proposal_scorer.py already read it. The `llm_config` entry points land in a follow-up task, so the call-site tests patch them with `monkeypatch.setattr(..., raising=False)` and pass either way. Verified: 11621 pass with `-n 12`. The single failure (test_bypass_report render assertion) reproduces on a sibling worktree at the same base commit, so it is pre-existing and unrelated. ruff check + ruff format clean. Co-authored-by: Cursor <cursoragent@cursor.com>
…ted llm module The package docstring carved out httpx "for the ``llm`` submodule", which this branch deleted. llm_config is now the single owner of provider transport for the whole repository, so it is the module the carve-out has to name. Co-authored-by: Cursor <cursoragent@cursor.com>
Normalize legacy JSON wrappers for every framework while rejecting argv values that Magpie cannot represent safely. Preserve JSON during deduplication so unrelated duplicate flags are still collapsed instead of reaching vLLM. Co-authored-by: Cursor <cursoragent@cursor.com>
…ei/forge-codex-provider
Hyperloom serves three deployment shapes, but resolve_openai_client_config() only ever read the OpenAI side. In an Anthropic-only deployment every single-shot OpenAI-protocol call site therefore failed to configure: the proposal scorer raised, audit refinement and the framework ranker degraded to their static paths, and only the critic worked because it carries its own Anthropic transport. Wire the fallback that derive_openai_base_url() was written for and that KernelForge's resolve_codex_gateway() already implements. When the OpenAI-side base URL is absent, derive it from ANTHROPIC_BASE_URL; when no OpenAI-side key is set, fall back to ANTHROPIC_AUTH_TOKEN then ANTHROPIC_API_KEY. Explicit OpenAI-side configuration still wins for key and URL independently, so codex-only and dual-configured deployments resolve byte-for-byte as before. The LLMConfigError now names every variable actually searched, and derive_openai_base_url's docstring no longer claims a caller-side precedence contract that no caller implemented. Add the Anthropic half of the gateway contract: get_anthropic_client / get_async_anthropic_client own the base URL, x-api-key, anthropic-version and ANTHROPIC_CUSTOM_HEADERS expansion, while anthropic_messages / aanthropic_messages own the POST to /v1/messages and flatten the reply into AnthropicMessageResult(text, stop_reason, usage). Clients stay uncached, errors propagate, and the caller keeps latency measurement. anthropic-version is now DEFAULT_ANTHROPIC_VERSION, defined and exported once, with ANTHROPIC_CUSTOM_HEADERS merged last so a gateway pinned elsewhere can override it. Migrate CriticAgentBackend's Anthropic path onto that contract. Its explicit anthropic_base_url / anthropic_api_key fields are overlaid on the environment so they keep precedence over ambient ANTHROPIC_*, and usage accounting plus failure tracing are unchanged. The three status / non-JSON / transport failures now arrive as one wrapped LLMCallFailed carrying the same detail, still with a failure trace row. _anthropic_text_from_content moves into llm_config with the call it served.
…forge-codex-provider # Conflicts: # src/hyperloom/inference_optimizer/cli/backends.py # src/hyperloom/inference_optimizer/tests/test_cli_backends_unit.py
…ei/forge-codex-provider
…aofei/forge-codex-provider # Conflicts: # src/hyperloom/common/llm_config.py # src/hyperloom/inference_optimizer/tests/test_llm_config.py
Reject fragmented quoted operands before deduplication and preserve independent environment overrides when unsafe server arguments are dropped. Co-authored-by: Cursor <cursoragent@cursor.com>
…s migrated The guard pinned 18 pre-existing violations so the migration could land in stages. All of them are gone: the TraceLens runner moved to the Codex Agent SDK, the RCA engines and the breakdown reporter moved onto the shared Anthropic contract, common/llm.py was deleted, and the codex / critic / proposal-scorer call sites now build their clients through llm_config. With the map empty the scan must find nothing, so any new ad-hoc provider client fails the build.
Routing the reporter through llm_config had moved its OpenAI path onto stream_chat_completion_text, which puts "stream": true on the wire. The reporter previously issued a plain chat completion, and switching the request shape was never part of moving client construction into llm_config. Add the sync twin of achat_completion and point the reporter at it, so the migration is transport-neutral. Both entry points now share one flattening helper. The reporter tests stubbed the contract with raising=False, which was there while the contract was still landing; with it in place that only hides a rename, so they now patch by real name.
The `if demangled and demangled != raw` guard is not needed when using itanium-demangler, which returns "" on failure (falsy). The original `if demangled:` guard works correctly with the new demangler. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove 3rdparty/composable_kernel/include from _CSRC_DIRS, restore /3rdparty/ to _SCAN_SKIP_MARKERS, and remove .hpp from native extension lists. These changes only affect CK kernels which are classified as aiter_ck (non_patchable) before any index lookup — the indexed source is never returned. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CI E2E report — ❌ Failed
|
kyle-hoffmeyer
changed the title
kyle-hoffmeyer
marked this pull request as ready for review
August 17, 2026 21:34
kyle-hoffmeyer
requested review from
a team,
Ahmedhasssan-aig,
devalshahamd and
tsrikris
as code owners
August 17, 2026 21:34
kyle-hoffmeyer
marked this pull request as draft
August 17, 2026 21:35
kyle-hoffmeyer
marked this pull request as ready for review
August 17, 2026 22:11
kyle-hoffmeyer
marked this pull request as draft
August 17, 2026 22:16
Collaborator
Author
|
I'm not sure if I added the itanium-demangler dependency correctly. Please take a look. |
kyle-hoffmeyer
marked this pull request as ready for review
August 18, 2026 23:11
Collaborator
|
Can you please try to pass all the CI tests? |
| def _cxxfilt_base(mangled: str) -> str: | ||
| """Demangle via ``c++filt`` when available (``""`` on failure). | ||
| def _demangle_itanium(mangled: str) -> str: | ||
| """Demangle an Itanium-mangled symbol via ``itanium-demangler``. |
Collaborator
There was a problem hiding this comment.
Can we keep the _cxxfilt_base (to use it as fallback) if itanium is not installed or missing for any reason?
Collaborator
|
@Ahmedhasssan-aig confirming that the redesign is acceptable to you? I see some of the components have been changed. @kyle-hoffmeyer Once you review the design with Ahmed, I'd suggest running a test E2E to make sure all the kernels are being mapped |
Collaborator
|
I reviewed the design with Kyle and the changes look reasonable to me. I left two comments for Kyle and recommended to pass all the CI tests. |
xiaofei-zheng
force-pushed
the
main
branch
2 times, most recently
from
August 25, 2026 10:29
f28ef6a to
af1d2c4
Compare
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
18 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.
Summary
This PR adds support for kernel source mapping for xDiT diffusion workloads. Each fix addresses a specific failure observed during source resolution of device kernels in these traces.
Change 1: Replace
c++filtwithitanium-demanglerFile:
source_resolver.py—_demangle_itanium()replacing_cxxfilt_base()Problem:
c++filtis the tool used to demangle kernel names. An example of a mangled kernel name:These kernel names are very difficult to parse but can be demangled to more easily parse.
c++filt(GNU Binutils) fails on xDiT workloads due to deeply nested CK convolution kernel symbols (~20+ levels of template nesting, 1575+ characters). When demangling fails,c++filtreturns the input string unchanged — an ambiguous failure signal that causes_non_patchable_kind()to misclassify CK kernels as patchable.itanium-demanglerhandles deeper nesting (no hardcoded recursion limits) and returns""on failure (unambiguous). This makes the existingif demangled:guard in_non_patchable_kind()work correctly —""is falsy, so the mangled regex fallback fires and CK kernels are correctly classified asaiter_ck.xDiT example:
aten::miopen_convolutiondispatches to CK grouped convolution:14 CK convolution kernels across 4 xDiT traces are affected.
Comparison:
c++filt(GNU Binutils)itanium-demangler(Python)""(clear)shutil,subprocessImportErrorfallback)Change 2: Fix
_base_from_demangled()for(anonymous namespace)::kernelsFile:
source_resolver.py—_base_from_demangled()Problem:
_base_from_demangled()is the function used to parse a demangled kernel name._base_from_demangled()usedre.split(r"[(<]", ...)to extract the kernel name. For signatures containing(anonymous namespace), the(in the namespace qualifier was matched first, truncating the name to"void".Fix: Rewrite
_base_from_demangled()to match the logic in_bypass_source_resolver._demangle_kernel_name(), which already handles this case correctly. Both functions now use the same approach for extracting the bare function identifier: stripvoidprefix, strip(anonymous namespace)::, then remove template/function args. Ideally this shared logic would live in a common utility function, but that belongs to a broader cleanup PR.Change 3: Add Tensile and MIOpen non-patchable classification
File:
source_resolver.py—_non_patchable_kind()Problem: Tensile GEMM kernels (
Cijk_*) and MIOpen convolution kernels were reported as"unresolved"with no explanation. These kernels are written in GPU assembly and shipped pre-compiled — no source code exists.Fix:
_non_patchable_kind()now accepts anop_nameparameter. Tensile is identified by device kernel name prefix (Cijk_*). MIOpen is identified by op name ("miopen" in op_name) rather than device kernel name patterns, which vary across MIOpen versions (e.g.,igemm_*,batched_transpose_*,Im2d2Col,Im3d2Col,SubTensorOpWithScalar*). Using the op name catches all MIOpen kernels regardless of the device kernel naming convention.xDiT examples:
Tensile GEMMs (~37% of xDiT device kernels):
MIOpen convolutions (all caught via op name
aten::miopen_convolution):These are now reported as
"non_patchable"with a reason string instead of bare"unresolved".Change 4: Mark Inductor-generated Triton kernels as non-patchable
File:
_bypass_source_resolver.py—resolve_triton_py()File:
_bypass_report.pyProblem:
torch.compileInductor-generated Triton kernels are not patchable as the source for them is generated at compile time within Inductor. These kernels were previously reported as"unresolved"fromresolve_triton_py. Tier 1 correctly identified them as non-editable (viatorchinductorin thekernel_filepath), but returned"unresolved"instead of"non_patchable", causing tiers 2 and 3 to run unnecessarily.Fix: When
resolve_triton_py()has akernel_filepath buteditable_trace_source()rejects it, return"non_patchable"instead of"unresolved". The framework scan/repo scan gates in_bypass_report.pynow usesource_method == "unresolved", so only genuinely unresolved kernels fall through to tiers 2 and 3.xDiT example: Inductor-generated Triton kernels:
Verification
Tested via the full pipeline (
analyze_trace(top_k=0)→build_candidates(top_k=0)) intracelens-xdit:v26.7(TraceLens patched xDiT image) against 4 xDiT diffusion workloads. Traces can be found in xDiT folders in https://github.com/AMD-AGI/TraceLens/tree/main/tests/traces/inference.Cijk_*)unresolvednon_patchable(tensile_precompiled)igemm_*)unresolvednon_patchable(miopen_precompiled)batched_transpose_*)unresolvednon_patchable(miopen_precompiled)_ZN2ck...)unresolved(c++filt failed silently)non_patchable(aiter_ck)triton_poi_*)unresolved(framework + repo scan wasted)non_patchable(framework + repo scan skipped)unresolved(_base_from_demangledreturned"void")resolved→groupnorm.curesolvedresolved(unchanged)New dependency
itanium-demangler(pip package, pure Python, no system deps). Added topyproject.tomlruntimeoptional dependencies. When not installed, a warning is logged ("itanium-demangler is not installed. Kernel classification may be degraded.") and demangling falls back to the mangled regex path.