fix: persist selected mod ID across Compat Kit audits - #84
Conversation
|
@codex review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for multi-mod NeoForge jars in the compatibility kit by adding a --mod-id option to the scan command. This allows users to target a specific mod when multiple are declared, which is then persisted and used during validation, migration, and diffing. The selected mod ID is also incorporated into the cache identity to isolate audits. Feedback on the changes highlights a cache key mismatch issue where scanning a single-mod jar without an explicit --mod-id computes a different cache key than subsequent diff or migrate-audit operations (which pass the mod ID explicitly). This leads to unexpected cache misses, and it is recommended to always include the resolved mod ID in the cache identity calculation.
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0244fa4d82
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Jar-backed diff was reading the old audit mod ID before schema validation, so malformed JSON threw KeyError instead of the normal exit-2 CLI error. Align the authoritative cache docs with resolved mod ID identity. via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
ce738e2 to
c27eada
Compare
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
scan --mod-idfor a target jar with multiple NeoForge[[mods]]entries.migrate-audit, jar-backeddiff, and complete exact-artifact validation.origin/main(5d757d7, including merged fix: handle nested classes under anonymous owners #82 scanner-format-17 / nested-owner work). Latest head:c27eada4944ab669ed4f539a55510dd0b6e6119d.Strict TDD
The original selected-mod regressions failed on pre-fix main because selection was not propagated through API/CLI, migration, diff, validation, or cache identity. A later review regression failed because malformed unselected metadata was silently ignored. A Codex review then required jar-backed
diffto call_validate_audit(old)before reading the old audit mod ID, and authoritative cache docs to name jar SHA, scanner format, and resolved target mod ID together.After the fixes and post-#82 rebase:
PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover scripts: 582/582 passed;./gradlew build --console=plain --no-daemonviaheavy_gradle.py: BUILD SUCCESSFUL;py_compile, andgit diff --check: passed.Documentation
docs/compat-kit.md,tools/compat-kit/README.md, anddocs/notes.mdfor selection, authoritative cache identity, migration/diff validation, and scanner format 17 coexistence with fix: handle nested classes under anonymous owners #82.Remaining risk