You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Looks fine to me, although should include some additional changes:
_build_eval_globals should include an entry for LLM. While this doesn't break anything right now, if someone was writing threats using this element then it would error out.
We should also include a mention in CHANGELOG when we next release as this is a breaking change and may catch out legacy users.
LLM is now in _build_eval_globals (2e40cf1). It was the only element class missing from the map, so conditions referencing it raised NameError even though eight shipped threats target the element.
The CHANGELOG entry is reworked in 97a4fbe. I'd originally filed the LLM addition itself under BREAKING, which was wrong — the breaking part is the strict rejection of unknown names. That's now its own entry describing what legacy users will actually hit (fail-fast when the Threat is constructed, instead of NameError later during evaluation) and how to resolve it, with the LLM fix moved under Fixed.
On the environment — fair. I have the suite running locally now. pytest tests/ gives 243 passed, 4 failed: test_dfd, test_dfd_colormap, test_dfd_duplicates_ignore and test_multilevel_dfd, all Windows path-separator diffs (pytm/images/... vs pytm\images\...). They fail identically on master, so they're not from this PR.
One thing I hit that may be worth its own issue: tests/test_pytmfunc.py can't be collected at all on Windows without PYTHONUTF8=1, because line 38 opens threats.json without an explicit encoding and cp1252 fails on it.
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
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
Testing
py -3.12 -m compileall pytm\threat.py tests\test_pydantic_models.pygit diff --checkpy -3.12 -m pytest tests\test_pydantic_models.py::TestConditionValidator -qin a temporary local venv