fix(model): stop tan model build corrupting non-path DRP-AI compile options (alp-sdk#1271) - #777
Draft
alpCaner wants to merge 2 commits into
Draft
fix(model): stop tan model build corrupting non-path DRP-AI compile options (alp-sdk#1271)#777alpCaner wants to merge 2 commits into
alpCaner wants to merge 2 commits into
Conversation
added 2 commits
August 15, 2026 18:33
…#1271)
model_cmd.py's hand-ported _resolve_compile resolved every string value
in a models[].compile.<backend> block to an absolute filesystem path,
even though only config/calibration/images/spec name paths. DRP-AI's
input_shape ("1,3,224,224"), input_name ("images") and product ("V2N")
were corrupted into filesystem paths before reaching the adapter, which
then made the adapter's own shape check misfire. alp-sdk fixed this as
issue #1271; tan's hand-ported copy never received it.
… e2e case, and fix two naming nits (tan-cli#776) Closes four gaps a review of 7d2b42b found in the alp-sdk#1271 port: - `test_resolve_compile_leaves_non_string_path_valued_options_unchanged` pins the `isinstance(v, str)` half of `_resolve_compile`'s guard -- a list/int-valued path key (`images: [a.png, b.png]`, `calibration: 100`) must pass through unchanged rather than raising `TypeError` at `Path.__truediv__`. Verified by temporarily dropping the isinstance check: the new case goes red with exactly that TypeError, then restored. - `test_compile_opts_paths_are_resolved_absolute_relative_to_board_dir`'s docstring no longer instructs the next porter to reintroduce the bug it once described (every string value becomes a path); it now says only `config`/`calibration`/`images`/`spec` do. - That same e2e case was blind to alp-sdk#1271 by construction (its only compile opt was a path key). Extended with DRP-AI's `input_shape`, `input_name` and `product` and asserted they survive verbatim through the `compileOpts` payload key, matching alp-sdk's own end-to-end pin (`test_alp_cli_model.py::test_alp_model_build_only_resolves_path_valued_drpai_opts`). - Filed tan-cli#776 for the shipped bug (changelog.d/781.fixed.md resolved to no real issue) and renamed the fragment to it. - Folded `test_model_cmd.py`'s two tests into `test_model_command.py` (preferred over renaming) -- `test_model_command.py` already exists for this command and all other command test modules are `test_<command>_command.py`. python -m pytest tests -q (isolated venv, python/): 4228 passed, 294 skipped, 1 xfailed, 0 failed.
alpCaner
marked this pull request as draft
August 15, 2026 19:47
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
tan model buildcorrupts every non-path DRP-AI compile option before it reachesthe adapter.
_resolve_compileresolved every string value in amodels[].compile.<backend>block to an absolute filesystem path, but only fourkeys name paths. So a
board.yamldeclaringreaches the DRP-AI adapter with
input_shaperewritten to/abs/path/to/project/1,3,224,224,input_nameto/abs/path/to/project/imagesand
productto/abs/path/to/project/V2N— which then makes the adapter's ownshape check misfire.
alp-sdk fixed this as alp-sdk#1271, restricting resolution to
_PATH_OPT_KEYS = {"config", "calibration", "images", "spec"}. tan's hand-portedcopy never received the fix. This PR ports it, and pins it.
Closes #776.
Why the pin did not catch it
python/tests/gates/test_planner_relocation_freshness.py:647pinsscripts/alp_cli/model.pyata51be0a8d3a16bd408bb57d01f049175406b73cc48ab9346d39555c3aa5b1925. That entrywas frozen at a post-#1271 upstream hash while tan's copy was still
pre-#1271, so the gate has been green for the whole life of the bug.
This is not specific to
model.py.HAND_PORT_HASHEShashes only theupstream side, so it proves the SDK file has not moved — never that tan
implements what was pinned. The other seven
scripts/alp_cli/*entries(
diagnostic_format,validate,new_som,doctor,explain,monitor,validator, lines 641-648) were frozen in the same commit on the samereasoning, so the same blindness applies to all eight. This PR does not fix
that class — it needs its own issue, and the audit needs to assert something
about tan's side (a recorded audit commit per entry, or a behavioural parity
test), not just the upstream hash.
Changes
python/tan/commands/model_cmd.py:128-151—_PATH_OPT_KEYSplus theupstream comment's reasoning, cited to
alp-sdk#1271. The comment is theload-bearing part: a future porter has to know why only four keys resolve.
python/tests/commands/test_model_command.py— three unit tests over_resolve_compile(non-path opts survive verbatim; non-string values under apath key survive;
None/{}pass through), plus the existing end-to-endbuild case extended with the three DRP-AI keys, asserting they survive through
the
"compileOpts"payload.changelog.d/776.fixed.md.Verification
Equivalence to the alp-sdk oracle was proven, not eyeballed. Review
extracted
_PATH_OPT_KEYS+_resolve_compilefromgit -C <alp-sdk> show origin/dev:scripts/alp_cli/model.pyby AST — notranscription — and ran both implementations against 24 inputs including
config: 123,images: ["a","b"],spec: {nested},config: Path(...),config: None/True/"", an already-absolute path, a../traversal, aCONFIGcase-variant,
{"drpai": None}, and falsy non-dicts. 0 divergences, keysets identical.
Both guards are mutation-pinned. Dropping
k in _PATH_OPT_KEYSreproducesAssertionError: assert '/tmp/.../1,3,224,224' == '1,3,224,224'. Droppingand isinstance(v, str)reproducesTypeError: unsupported operand type(s) for /: 'PosixPath' and 'list'— whichthe broad handler at
model_cmd.py:582would otherwise convert intomodel.internal-failureinstead of a build. Both restored;git diffclean.Why the existing e2e test missed this: its
board.yamldeclared onlyethos_u: {config: vela.ini}, andconfigis one of the four path keys, sopre-fix and post-fix produced byte-identical output. Extended here to cover the
DRP-AI keys, matching how alp-sdk pinned it
(
tests/scripts/test_alp_cli_model.py::test_alp_model_build_only_resolves_path_valued_drpai_opts).Gate — from
python/, in an isolated venv (import tanverified to resolveto this worktree),
env -u ALP_SDK_ROOT python -m pytest tests -q:Zero failures. Note for anyone reproducing: a bare system
python3.12on themaintainer box carries a stale
alp-taneditable install that leaks ontosys.pathand injects ~706 false failures — use a fresh venv.Scope
No behaviour change beyond the four path keys. No envelope-contract change, no
new dependency, no flash path touched. Independent of the ADR-0028 model-engine
relocation (alp-sdk#1470) — this fixes the shipped bug on its own.