From 7d2b42bb634a1b111ebaa8581a15ea05c75052bc Mon Sep 17 00:00:00 2001 From: Caner Alp Date: Sat, 15 Aug 2026 18:33:27 +0000 Subject: [PATCH 1/2] fix(model): stop resolving non-path compile options to paths (alp-sdk#1271) model_cmd.py's hand-ported _resolve_compile resolved every string value in a models[].compile. 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. --- changelog.d/781.fixed.md | 11 +++++++ python/tan/commands/model_cmd.py | 18 ++++++++--- python/tests/commands/test_model_cmd.py | 40 +++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 changelog.d/781.fixed.md create mode 100644 python/tests/commands/test_model_cmd.py diff --git a/changelog.d/781.fixed.md b/changelog.d/781.fixed.md new file mode 100644 index 00000000..c181eec1 --- /dev/null +++ b/changelog.d/781.fixed.md @@ -0,0 +1,11 @@ +- **`tan model build` no longer corrupts non-path compile options into + filesystem paths.** `model_cmd.py`'s `_resolve_compile` resolved every + string value in a `models[].compile.` block relative to + `board.yaml`'s directory, 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 being mangled into absolute + 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. Only `config`, `calibration`, `images` + and `spec` are resolved to paths now; every other compile option passes + through unchanged. diff --git a/python/tan/commands/model_cmd.py b/python/tan/commands/model_cmd.py index b803f5e4..2ad9015f 100644 --- a/python/tan/commands/model_cmd.py +++ b/python/tan/commands/model_cmd.py @@ -125,15 +125,25 @@ def __init__(self, code: str, message: str, exit_code: ExitCode) -> None: self.exit_code = exit_code +#: Compile-opt keys that name a filesystem path (resolved relative to +#: board.yaml). Not every value in a models[].compile. block is a +#: path -- e.g. drpai's input_shape ("1,3,224,224"), input_name ("images") and +#: product ("V2N") are opaque strings that must reach the adapter unchanged +#: (alp-sdk#1271: resolving them as paths corrupted a genuine shape string into +#: a filesystem path, which then made the adapter's own shape check misfire). +_PATH_OPT_KEYS = {"config", "calibration", "images", "spec"} + + def _resolve_compile(block: dict | None, base: Path) -> dict | None: - """Port of `model.py::_resolve_compile`: every string value in each - per-backend compile block becomes an absolute path relative to the - `board.yaml` dir -- every current opts value is a path.""" + """Port of `model.py::_resolve_compile`: resolve known path-valued keys + in each per-backend compile block to an absolute path relative to the + `board.yaml` dir; every other value (shape strings, node names, product + ids, ...) passes through unchanged (alp-sdk#1271).""" if not block: return None return { backend: { - k: (str((base / v).resolve()) if isinstance(v, str) else v) + k: (str((base / v).resolve()) if k in _PATH_OPT_KEYS and isinstance(v, str) else v) for k, v in (opts or {}).items() } for backend, opts in block.items() diff --git a/python/tests/commands/test_model_cmd.py b/python/tests/commands/test_model_cmd.py new file mode 100644 index 00000000..af5becb5 --- /dev/null +++ b/python/tests/commands/test_model_cmd.py @@ -0,0 +1,40 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`_resolve_compile` -- the `models[].compile.` path resolver. + +Pins alp-sdk#1271: only `config`/`calibration`/`images`/`spec` name paths. +`_resolve_compile` used to resolve EVERY string value in a compile block to an +absolute filesystem path, so DRP-AI's `input_shape` ("1,3,224,224"), +`input_name` ("images") and `product` ("V2N") -- opaque strings the adapter +must receive unchanged -- were corrupted into filesystem paths before ever +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. +""" +from pathlib import Path + +from tan.commands.model_cmd import _resolve_compile + + +def test_resolve_compile_leaves_non_path_options_unchanged(tmp_path): + """alp-sdk#1271: only `config`/`calibration`/`images`/`spec` name paths. + Resolving a shape string turned "1,3,224,224" into a filesystem path and + made the DRP-AI adapter's own shape check misfire.""" + out = _resolve_compile( + {"drpai": {"input_shape": "1,3,224,224", "input_name": "images", + "product": "V2N", "config": "cfg.json"}}, + tmp_path, + ) + assert out["drpai"]["input_shape"] == "1,3,224,224" + assert out["drpai"]["input_name"] == "images" + assert out["drpai"]["product"] == "V2N" + # the one genuine path key IS resolved, absolute, against board.yaml's dir + assert out["drpai"]["config"] == str((tmp_path / "cfg.json").resolve()) + + +def test_resolve_compile_passes_through_none_and_empty(): + """An absent `compile:` block and an empty one both fall through the + `if not block:` guard to `None` -- true of both the pre-fix tan code and + the upstream alp-sdk#1271 fix (`if not block: return None`, unchanged by + that fix); this is pre-existing, unrelated behaviour, not part of the + path-key drift this test file otherwise pins.""" + assert _resolve_compile(None, Path(".")) is None + assert _resolve_compile({}, Path(".")) is None From f8896f34ed18d68f1cb0971d6be2f7791a8e5cfc Mon Sep 17 00:00:00 2001 From: Caner Alp Date: Sat, 15 Aug 2026 18:55:32 +0000 Subject: [PATCH 2/2] fix(model): pin the isinstance guard, correct a docstring, extend the 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.py`. python -m pytest tests -q (isolated venv, python/): 4228 passed, 294 skipped, 1 xfailed, 0 failed. --- changelog.d/{781.fixed.md => 776.fixed.md} | 6 +- python/tests/commands/test_model_cmd.py | 40 ---------- python/tests/commands/test_model_command.py | 81 +++++++++++++++++++-- 3 files changed, 78 insertions(+), 49 deletions(-) rename changelog.d/{781.fixed.md => 776.fixed.md} (75%) delete mode 100644 python/tests/commands/test_model_cmd.py diff --git a/changelog.d/781.fixed.md b/changelog.d/776.fixed.md similarity index 75% rename from changelog.d/781.fixed.md rename to changelog.d/776.fixed.md index c181eec1..260cde8e 100644 --- a/changelog.d/781.fixed.md +++ b/changelog.d/776.fixed.md @@ -6,6 +6,6 @@ (`"images"`) and `product` (`"V2N"`) were being mangled into absolute 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. Only `config`, `calibration`, `images` - and `spec` are resolved to paths now; every other compile option passes - through unchanged. + hand-ported copy never received it (tan-cli#776). Only `config`, + `calibration`, `images` and `spec` are resolved to paths now; every other + compile option passes through unchanged. diff --git a/python/tests/commands/test_model_cmd.py b/python/tests/commands/test_model_cmd.py deleted file mode 100644 index af5becb5..00000000 --- a/python/tests/commands/test_model_cmd.py +++ /dev/null @@ -1,40 +0,0 @@ -# SPDX-License-Identifier: Apache-2.0 -"""`_resolve_compile` -- the `models[].compile.` path resolver. - -Pins alp-sdk#1271: only `config`/`calibration`/`images`/`spec` name paths. -`_resolve_compile` used to resolve EVERY string value in a compile block to an -absolute filesystem path, so DRP-AI's `input_shape` ("1,3,224,224"), -`input_name` ("images") and `product` ("V2N") -- opaque strings the adapter -must receive unchanged -- were corrupted into filesystem paths before ever -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. -""" -from pathlib import Path - -from tan.commands.model_cmd import _resolve_compile - - -def test_resolve_compile_leaves_non_path_options_unchanged(tmp_path): - """alp-sdk#1271: only `config`/`calibration`/`images`/`spec` name paths. - Resolving a shape string turned "1,3,224,224" into a filesystem path and - made the DRP-AI adapter's own shape check misfire.""" - out = _resolve_compile( - {"drpai": {"input_shape": "1,3,224,224", "input_name": "images", - "product": "V2N", "config": "cfg.json"}}, - tmp_path, - ) - assert out["drpai"]["input_shape"] == "1,3,224,224" - assert out["drpai"]["input_name"] == "images" - assert out["drpai"]["product"] == "V2N" - # the one genuine path key IS resolved, absolute, against board.yaml's dir - assert out["drpai"]["config"] == str((tmp_path / "cfg.json").resolve()) - - -def test_resolve_compile_passes_through_none_and_empty(): - """An absent `compile:` block and an empty one both fall through the - `if not block:` guard to `None` -- true of both the pre-fix tan code and - the upstream alp-sdk#1271 fix (`if not block: return None`, unchanged by - that fix); this is pre-existing, unrelated behaviour, not part of the - path-key drift this test file otherwise pins.""" - assert _resolve_compile(None, Path(".")) is None - assert _resolve_compile({}, Path(".")) is None diff --git a/python/tests/commands/test_model_command.py b/python/tests/commands/test_model_command.py index 6a24fb08..75d0e981 100644 --- a/python/tests/commands/test_model_command.py +++ b/python/tests/commands/test_model_command.py @@ -345,13 +345,71 @@ def test_a_failed_model_is_an_issue_not_a_traceback_and_the_batch_continues(tmp_ assert "no blob compiled" in doc["issues"][0]["message"] +# -------------------------------------------------------------------------- +# `_resolve_compile` -- the `models[].compile.` path resolver, unit +# -------------------------------------------------------------------------- +# +# Pins alp-sdk#1271: only `config`/`calibration`/`images`/`spec` name paths. +# `_resolve_compile` used to resolve EVERY string value in a compile block to +# an absolute filesystem path, so DRP-AI's `input_shape` ("1,3,224,224"), +# `input_name` ("images") and `product` ("V2N") -- opaque strings the adapter +# must receive unchanged -- were corrupted into filesystem paths before ever +# 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 until tan-cli#776. + + +def test_resolve_compile_leaves_non_path_options_unchanged(tmp_path): + """alp-sdk#1271: only `config`/`calibration`/`images`/`spec` name paths. + Resolving a shape string turned "1,3,224,224" into a filesystem path and + made the DRP-AI adapter's own shape check misfire.""" + out = model_cmd._resolve_compile( + {"drpai": {"input_shape": "1,3,224,224", "input_name": "images", + "product": "V2N", "config": "cfg.json"}}, + tmp_path, + ) + assert out["drpai"]["input_shape"] == "1,3,224,224" + assert out["drpai"]["input_name"] == "images" + assert out["drpai"]["product"] == "V2N" + # the one genuine path key IS resolved, absolute, against board.yaml's dir + assert out["drpai"]["config"] == str((tmp_path / "cfg.json").resolve()) + + +def test_resolve_compile_leaves_non_string_path_valued_options_unchanged(tmp_path): + """A path-valued key (`images`) can still carry a non-string value in a + plausible `board.yaml` spelling -- a YAML flow-sequence + (`images: [a.png, b.png]`) or a stray int (`calibration: 100`). Those must + pass through unchanged rather than reaching `Path.__truediv__`, which + raises `TypeError: unsupported operand type(s) for /: 'PosixPath' and + 'list'` for a non-str/PathLike operand -- caught by the broad handler at + `model_cmd.py`'s driver-spawn callsite and turned into + `model.internal-failure` / exit `INTERNAL_FAILURE` instead of a build. + Guards the `isinstance(v, str)` half of `_resolve_compile`'s guard, not + just the `k in _PATH_OPT_KEYS` half that the test above pins.""" + out = model_cmd._resolve_compile({"drpai": {"images": ["a", "b"]}}, tmp_path) + assert out["drpai"]["images"] == ["a", "b"] + + +def test_resolve_compile_passes_through_none_and_empty(): + """An absent `compile:` block and an empty one both fall through the + `if not block:` guard to `None` -- true of both the pre-fix tan code and + the upstream alp-sdk#1271 fix (`if not block: return None`, unchanged by + that fix); this is pre-existing, unrelated behaviour, not part of the + path-key drift this section otherwise pins.""" + assert model_cmd._resolve_compile(None, Path(".")) is None + assert model_cmd._resolve_compile({}, Path(".")) is None + + @pytest.mark.skipif(not _HAS_PYTHON, reason="no python interpreter available to spawn") def test_compile_opts_paths_are_resolved_absolute_relative_to_board_dir(tmp_path): - """Port of `model.py::_resolve_compile`: string opt values become absolute - paths relative to board.yaml's own directory. Also covers the two other - values `build_model` uses to choose which silicon to compile for -- `sku` - and `metadata_root` -- untested before: get either wrong and the driver - silently compiles blobs for the wrong part.""" + """Port of `model.py::_resolve_compile`: only PATH-VALUED opt keys + (`config`/`calibration`/`images`/`spec`) become absolute paths relative to + board.yaml's own directory -- every other compile option (DRP-AI's + `input_shape`/`input_name`/`product` among them, alp-sdk#1271) must + survive verbatim. Also covers the two other values `build_model` uses to + choose which silicon to compile for -- `sku` and `metadata_root` -- + untested before: get either wrong and the driver silently compiles blobs + for the wrong part.""" sdk = make_sdk(tmp_path / "sdk") write( tmp_path / "sdk" / "scripts" / "alp_model" / "__init__.py", "" @@ -384,7 +442,11 @@ def build_model(*, sku, name, source, out_dir, metadata_root, compile_opts=None) " source: source.tflite\n" " compile:\n" " ethos_u:\n" - " config: vela.ini\n", + " config: vela.ini\n" + " drpai:\n" + " input_shape: \"1,3,224,224\"\n" + " input_name: images\n" + " product: V2N\n", ) result = runner.invoke( app, @@ -400,6 +462,13 @@ def build_model(*, sku, name, source, out_dir, metadata_root, compile_opts=None) assert opts["sku"] == "E1M-TEST" assert opts["metadataRoot"] == str(sdk / "metadata") assert opts["compileOpts"]["ethos_u"]["config"] == str((tmp_path / "vela.ini").resolve()) + # alp-sdk#1271 / tan-cli#776: these three DRP-AI opts are opaque strings, + # not path keys -- they must survive the round trip through + # `_resolve_compile` byte-for-byte, not get mangled into filesystem paths. + drpai = opts["compileOpts"]["drpai"] + assert drpai["input_shape"] == "1,3,224,224" + assert drpai["input_name"] == "images" + assert drpai["product"] == "V2N" # --------------------------------------------------------------------------