Skip to content

fix(provenance): record the AITER that is actually installed - #1508

Open
rpoornac wants to merge 1 commit into
mainfrom
fix/rpoornac/aiter-dist-name
Open

rpoornac wants to merge 1 commit into
mainfrom
fix/rpoornac/aiter-dist-name

Conversation

@rpoornac

@rpoornac rpoornac commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Problem

stack_fingerprint["aiter"] was untrustworthy for two independent reasons. Thanks to @ZhengGong-amd, whose review replaced this PR's original approach — the earlier revision treated the distribution name as a dual name and kept a candidate list, which was the wrong shape for the fix.

Nothing wrote the env keys it read. The tuple was ("AITER_COMMIT", "AITER_VERSION"), and no code in this repo sets either, so the env path never produced a value. Meanwhile install_baremetal.sh already resolves the exact tag it installs, exports it (line 636), and persists it to .env as AITER_REF (line 2039) — which the dotenv loader admits via its AITER_ prefix allowlist:

is_allowed_dotenv_key('AITER_REF') = True

The value was sitting one key away. AITER_REF joins the tuple behind AITER_COMMIT. A git tag is finer than a wheel version, which is what aiter_preflight's soft warning asks for, and it is the only route that works on the default isolated vLLM path, where aiter lives in the framework venv and no in-process probe can reach it under any name.

The probe used a name AITER stopped using. PACKAGE_NAME in ROCm/aiter is aiter at v0.1.6 and v0.1.7, and amd-aiter from v0.1.8 onward. So the lookup missed every host on v0.1.8 or newer. Worse, on PyPI aiter is an unrelated 2019 async-iterator library:

name=aiter  version=0.13.20191203
summary=Useful patterns building upon asynchronous iterators.

Wherever that package is installed, the probe recorded 0.13.20191203 as the AITER version — verified against main, not hypothetical. The name is corrected rather than kept as a fallback, precisely so that value can no longer be produced: recording nothing beats recording something that looks like an answer. Hosts older than v0.1.8 are covered by AITER_REF, which is exact.

Change

Two lines of behaviour:

"aiter": ("AITER_COMMIT", "AITER_VERSION", "AITER_REF"),
dist = {"sglang": "sglang", "vllm": "vllm", "aiter": "amd-aiter"}.get(component)

The rest is comments recording why the old name is not kept. The venv branch is back to main's form: it is unreachable for aiter, since _framework_site_packages requires RESOLVED_FRAMEWORK == component and preflight.py:929 only ever sets that to the serving framework.

Only what gets written changes. No read path compares rocm or aiter against the pod, so warm-start selection and replay are unaffected.

Why this is separate from the comparison gap

The read-side gap is tracked in #1507. This is a prerequisite: a comparison fed 0.13.20191203 would report a confident mismatch against every real AITER build, and one fed unknown on a whole class of hosts would be silently inert. That failure mode is not hypothetical — it was defect #3 in the review of #1495.

An aside found while checking the allowlist: is_allowed_dotenv_key('ROCM_VERSION') is False, so the ROCm half of the fingerprint has no .env route at all. Out of scope here; noted on #1507.

Validation

Four tests, each confirmed to fail on main:

Test On main
AITER_REF reaches the fingerprint unknown
the renamed distribution is found unknown
the PyPI aiter is never recorded as AMD's 0.13.20191203
AITER_COMMIT outranks the installer ref passes — regression guard

The stubs answer every component explicitly rather than raising KeyError into a broad except, so all four pass unchanged if that catch is later narrowed to PackageNotFoundError — verified both ways.

test_common_provenance.py and test_aiter_preflight.py together 61 passed. ruff check and ruff format --check clean.

@ZhengGong-amd

Copy link
Copy Markdown
Collaborator

The bug is real, but it is a rename, not a dual name: ROCm/aiter changed PACKAGE_NAME from aiter to amd-aiter at v0.1.8 (v0.1.7 still says aiter). So the old table had the name wrong, and the minimal fix is one word — "aiter": "amd-aiter" — rather than a candidate list.

I'd drop the aiter fallback rather than keep it. On PyPI that name belongs to an unrelated 2019 async-iterator library, so this path can record 0.13.20191203 as the AITER version — precisely what _probe_pkg_version's own docstring calls "worse than unknown, because it looks right". aiter_preflight gets away with two names only because it writes dist==version; this writes a bare version.

More importantly, the probe is the wrong layer. install_baremetal.sh already resolves the exact tag it installed, exports it, and persists it to .env (AITER_REF, lines 617 and 1988), and the session sources .env under set -a (SKILL.md:188). But _STACK_FINGERPRINT_ENVS reads AITER_COMMIT / AITER_VERSION, which nothing in this repo writes. One line closes the gap:

"aiter": ("AITER_COMMIT", "AITER_VERSION", "AITER_REF"),

That records a git tag (finer than a wheel version — exactly what aiter_preflight's soft warning asks for), is immune to the rename, and also covers the default vLLM isolated path, where aiter lives in $VLLM_VENV_ROOT and no in-process probe can reach it under any name.

Two smaller things. The venv branch is unreachable for aiter today — _framework_site_packages requires RESOLVED_FRAMEWORK == component and preflight only ever sets it to vllm/sglang — so the declared-order rewrite is a no-op. And test_a_host_with_both_aiter_distributions_records_one_of_them_deterministically passes only because the broad except Exception swallows the KeyError its own stub raises when detect_stack_fingerprint probes sglang; narrowing the catch to PackageNotFoundError would turn it red.

@rpoornac

Copy link
Copy Markdown
Collaborator Author

All five points confirmed, and the PR is reworked to your design. Pushed as da43d43. I checked each one rather than taking it on trust, because two of them change what the fix should be.

1. It is a rename, not a dual name — confirmed at the source. PACKAGE_NAME in ROCm/aiter:

tag PACKAGE_NAME
v0.1.6 aiter
v0.1.7 aiter
v0.1.8 amd-aiter
v0.1.9, main amd-aiter

So the old table had the name wrong and a candidate list was the wrong shape for the fix. It is now one word: "aiter": "amd-aiter". Your v0.1.8 boundary is exact.

2. The aiter fallback is actively harmful — and it is worse than a latent risk. PyPI confirms your read:

name=aiter  version=0.13.20191203
summary=Useful patterns building upon asynchronous iterators.
home=https://github.com/richardkiss/aiter

So I wrote a test asserting that version can never be recorded, and ran it against main:

>       assert _prov.detect_stack_fingerprint({}, probe=True)["aiter"] == "unknown"
E       AssertionError: assert '0.13.20191203' == 'unknown'

main records the async-iterator library's version as the AITER version today, wherever that package is installed. That is a live fault, not just a hazard my PR would have introduced, and dropping the fallback is what closes it. One correction: the phrase "worse than unknown, because it looks right" does not appear in _probe_pkg_version's docstring or anywhere in the repo — but the argument stands on its own, so I put that reasoning in the comment where you'd expect to find it.

3. AITER_REF is the right layer — confirmed end to end. install_baremetal.sh selects and exports the tag (line 636) and persists it with upsert_dotenv_var AITER_REF (line 2039; your 617/1988 are close enough that we're looking at the same code). The part I checked because it would have sunk the idea is the gate, and it passes:

is_allowed_dotenv_key('AITER_REF') = True   # via the AITER_ prefix allowlist

So the value really does reach os.environ. The tuple is now ("AITER_COMMIT", "AITER_VERSION", "AITER_REF") in your order. One detail: the .env is loaded by the Python preflight's dotenv step, not by shell set -a — I could not find set -a in SKILL.md or the assets — but the path works, which is what matters.

An aside from checking that allowlist: is_allowed_dotenv_key('ROCM_VERSION') is False, so the ROCm half of the fingerprint has no .env route at all. Not this PR's business; noted for #1507.

4. The venv branch is unreachable for aiter — confirmed. _framework_site_packages returns None unless RESOLVED_FRAMEWORK == component, and preflight.py:929 only ever sets it to the serving framework. My declared-order rewrite there was dead code, so it is gone; that branch is back to main's form untouched. This is also why the isolated vLLM path needs AITER_REF rather than a cleverer probe.

5. The test passed because of the broad except — confirmed, exactly as you described. Narrowing the catch:

E   KeyError: 'sglang'
1 failed, 2 passed

My stub raised KeyError for the components it didn't list and except Exception swallowed it. The replacement stub answers every component explicitly, raising PackageNotFoundError for anything absent, so the tests now pass unchanged whether the catch is broad or narrowed to PackageNotFoundError — I verified both.

The business-code diff is down to two lines plus comments. Four tests, each confirmed to fail on main: AITER_REF recorded (unknown before), the renamed distribution found (unknown before), the PyPI collision refused (0.13.20191203 before), and AITER_COMMIT still outranking the ref.

Also: #1495 is re-landed as #1542 per your recommendation, rebased onto current main.

@rpoornac rpoornac changed the title fix(provenance): record AITER installed under either distribution name fix(provenance): record the AITER that is actually installed Sep 17, 2026
Two independent faults made stack_fingerprint["aiter"] untrustworthy.

The env tuple read AITER_COMMIT and AITER_VERSION, neither of which
anything in this repo writes, so the env path never produced a value.
install_baremetal.sh already resolves the exact tag it installs, exports
it, and persists it to .env as AITER_REF, which the dotenv loader admits
under its AITER_ prefix -- the value was one key away the whole time.
AITER_REF joins the tuple behind AITER_COMMIT. It also covers the default
isolated vLLM path, where aiter lives in the framework venv and no
in-process probe can reach it under any name.

The probe then looked up a distribution named "aiter", but AITER renamed
itself to "amd-aiter" at v0.1.8 (v0.1.7 still says "aiter"), so the
lookup missed every host running v0.1.8 or newer. Worse, on PyPI "aiter"
is an unrelated 2019 async-iterator library, so where that package was
installed the probe recorded its version, 0.13.20191203, as the AITER
version. The name is corrected rather than kept as a fallback, precisely
so that value can no longer be produced: recording nothing is better than
recording something that looks like an answer. Hosts older than v0.1.8
are covered by AITER_REF, which is exact.

Only the written value changes; no read path compares rocm or aiter
against the pod today. That gap is tracked in #1507, and this is a
prerequisite for it -- a comparison fed 0.13.20191203 would report a
confident mismatch against every real AITER build.

The tests answer every component explicitly rather than letting a stub
raise KeyError into a broad except, so they pass unchanged if that catch
is later narrowed to PackageNotFoundError.

Co-authored-by: Cursor <cursoragent@cursor.com>
@rpoornac
rpoornac force-pushed the fix/rpoornac/aiter-dist-name branch from da43d43 to 084fd33 Compare September 17, 2026 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants