Repository navigation
Conversation
Widen [compat] to OrderedCollections = "1.8, 2" and Parameters = "0.12, 0.13", and move the tracked manifests to OrderedCollections 2.0.3 and Parameters 0.13.1. KiteUtils moves 0.13.1 -> 0.13.3 with them: 0.13.0-0.13.2 cap OrderedCollections at 1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1-Bort-1
commented
Oct 27, 2026
1-Bort-1
left a comment
Member
Author
There was a problem hiding this comment.
Independent review (advisory)
Verdict: APPROVE WITH COMMENTS · 1 inline, 0 off the diff
Good
- Compat widening is the smallest change that does the job:
OrderedCollections = "1.8, 2"andParameters = "0.12, 0.13"keep the old lower bounds as the card says (Project.toml:80, :84) - The manifest changes match the card exactly: in both the 1.12 and 1.13 manifests only KiteUtils, OrderedCollections, Parameters and project_hash move (checked hunk by hunk, and
git show --stat HEADshows 14 lines per manifest) - Moving KiteUtils to 0.13.3 is explained by the reproduced resolver error, and it stays inside the existing
KiteUtils = "0.13.1"compat entry (Project.toml:71), so that entry rightly stays unchanged - Package code depends on OrderedCollections only through
OrderedDict(src/definition_loader.jl:4,397; src/system_structure/definition.jl; src/kernel_backend/codegen/mtk_codegen.jl:851), and on Parameters only through@with_kw; both survive the major bump, and the card's tests reach the YAMLdicttype=OrderedDictpath - The release is kept out of this PR and points at the bump PR #432, as §9 requires; test/Project.toml has no compat entries for these packages, so nothing there needed widening
- The card is honest about what was not run (Julia 1.12 tests, reuse_lint, full suite, CI)
Not good
changelog.d/431-allow-parameters-0-13-and-orderedcollect.md:2— The clause "is no longer held at Parameters 0.12 and OrderedCollections 1" is a countermand (§7) that only makes sense next to the old bounds; state what is true instead, e.g. "SymbolicAWEModels allows Parameters 0.12–0.13 and OrderedCollections 1.8–2."- CI now tests only the new releases, so the old bounds the card says still work (Parameters 0.12, OrderedCollections 1.8) are no longer tested anywhere; a downgrade or minimum-compat job would back that claim
- Only three test files were run on 1.13, and the codegen path in mtk_codegen.jl:851 (
OrderedDictof symbolic eqs) is the most likely place for OrderedCollections 2 to differ; wait for the full suite or CI before merging - The 1.12 manifest was resolved but never loaded, even though it is a tracked default that CI and users install from
claude, rubric CLEAN_CODE.md. A different lab from the implementer
on purpose: a reviewer sharing its blind spots would not flag its mistakes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1-Bort-1
marked this pull request as ready for review
October 27, 2026 12:36
Member
Author
|
Local full suite: PASS (29 min, Julia 1.13.0, one cell of the matrix) |
Both sides changed Project.toml, so the manifests conflicted only on project_hash. Re-resolved both under their Julia versions; nothing moved but the hash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
SymbolicAWEModels capped Parameters at 0.12 and OrderedCollections at 1. Because of that, every environment using it, such as RobustKiteControllers.jl and V3Kite.jl, stayed on the older releases even though those packages already allow the new ones. Both new versions are now allowed, and the old ones stay allowed.
The tracked manifests now pin the new releases, so CI tests this package against them. Widening the bounds alone would not have done that: the old pins still satisfied the bounds, so a resolve would have kept them. Only three packages move: Parameters 0.12.3 → 0.13.1, OrderedCollections 1.8.2 → 2.0.3, and KiteUtils 0.13.1 → 0.13.3. KiteUtils has to move because 0.13.0 to 0.13.2 cap OrderedCollections at 1, and 0.13.3 lifts that cap. KiteUtils stays within the 0.13 series this package already allows, so its compat entry is unchanged. Every other package in the manifest already allows both new versions.
Only the new versions are tested: CI installs from the tracked manifests, which now pin them. No CI job installs the oldest allowed versions, so Parameters 0.12 and OrderedCollections 1.8 remain allowed but untested. Adding such a job is a separate change.
This change does not tag the patch release the issue asks for. The release is a separate bump pull request, and #432 (v0.21.0) is already open for one.
Verification
OrderedCollections … restricted by compatibility requirements with KiteUtils to versions: 1.6.3 - 1.8.2, which is why KiteUtils 0.13.3 comes alongtest/test_structure_document.jl61/61 ·test/test_definition_models.jl45/45 ·test/test_deserialization.jl10/10 ·test/test_backend_parity.jl3/3./bin/reuse_lint(thereusetool is not installed on the build machine)Opened by
1-Bort-1, an AI agent working for @1-Bart-1.Closes #431 · task
SymbolicAWEModels.jl-431