compat: add Create Crafts & Additions deterministic recipe integration - #74
Conversation
![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 compatibility support for the Create Crafts & Additions mod, enabling integration with its Rolling Mill and Tesla Coil charging recipes while explicitly excluding Liquid Burning. It adds a dedicated GameTest fixture with eight integration tests, updates the overall compatibility matrix to include fourteen mods (raising the benchmark to 11,779 recipes), and refactors the Compat Kit tool to bundle runtime dependencies into the compile classpath. Feedback on the implementation highlights two critical robustness issues in CreateadditionCompat.java: a potential IndexOutOfBoundsException in itemKeys caused by redundant deduplication, and a potential division-by-zero ArithmeticException in chargingWork if the charge rate evaluates to zero.
| private static List<StorageResourceKey> itemKeys( | ||
| List<ItemStack> representatives, | ||
| HolderLookup.Provider registries | ||
| ) { | ||
| return representatives.stream() | ||
| .map(stack -> StorageResourceKey.item(stack, registries)) | ||
| .distinct() | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
Using .distinct() on the StorageResourceKey stream in itemKeys is redundant because representatives is already distinct. More importantly, if any normalization in StorageResourceKey.item causes two distinct ItemStacks to map to the same key, the size of alternatives will be smaller than representatives. This will result in an IndexOutOfBoundsException when accessing alternatives.get(index) in consumedWithRemainder. Removing .distinct() ensures the list sizes always match and prevents potential runtime crashes.
private static List<StorageResourceKey> itemKeys(
List<ItemStack> representatives,
HolderLookup.Provider registries
) {
return representatives.stream()
.map(stack -> StorageResourceKey.item(stack, registries))
.toList();
}| private static long chargingWork(ChargingRecipe recipe) { | ||
| long rate = chargeRate(recipe); | ||
| long energy = recipe.getEnergy(); | ||
| return Math.addExact(energy, rate - 1L) / rate; | ||
| } |
There was a problem hiding this comment.
If the createaddition configuration is reloaded or modified at runtime such that chargeRate(recipe) returns 0 or a negative value, chargingWork will throw an ArithmeticException due to division by zero. Guarding against non-positive rates by clamping rate to at least 1L (and energy to at least 0) ensures robustness against dynamic config reloads and prevents server crashes.
| private static long chargingWork(ChargingRecipe recipe) { | |
| long rate = chargeRate(recipe); | |
| long energy = recipe.getEnergy(); | |
| return Math.addExact(energy, rate - 1L) / rate; | |
| } | |
| private static long chargingWork(ChargingRecipe recipe) { | |
| long rate = Math.max(1L, chargeRate(recipe)); | |
| long energy = Math.max(0L, recipe.getEnergy()); | |
| return (energy + rate - 1L) / rate; | |
| } |
e9a3a43 to
34aeec7
Compare
34aeec7 to
acd9c18
Compare
|
@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: acd9c1851b
ℹ️ 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".
| private static boolean supportsRolling(RollingRecipe recipe) { | ||
| return rollingDuration() > 0 | ||
| && exact(recipe.getIngredient()) | ||
| && !recipe.getResultStack().isEmpty(); |
There was a problem hiding this comment.
When a datapack adds a RollingRecipe with multiple ingredients or probabilistic/multiple results, this predicate still accepts it because it examines only getIngredient() and getResultStack(). rollingPlan then consumes only that ingredient and emits only that stack, allowing Auto Storage to produce a different, potentially guaranteed result from the actual recipe. Enforce the contract's single-ingredient, no-fluid, single-100%-output shape using the inherited processing-recipe collections before exposing the recipe.
Useful? React with 👍 / 👎.
| "catalyst_tool_remainder_exact": [ | ||
| { | ||
| "marker": "Create Crafts & Additions charging did not consume exact FE/work", | ||
| "source": "src/createadditionFixture/java/com/swear/autostorage/fixture/createaddition/CreateadditionIntegrationGameTests.java", | ||
| "task": "runCreateadditionGameTestServer" |
There was a problem hiding this comment.
When consumedWithRemainder drops or misassigns an alternative-specific crafting remainder, this evidence still passes: the mapped charging test consumes a gold nugget, which has no crafting remainder, and never asserts any returned remainder. Compat Kit can therefore report catalyst_tool_remainder_exact as passed without testing that behavior; map this check to an actual remainder-bearing recipe/assertion or add one to this fixture.
AGENTS.md reference: AGENTS.md:L176-L176
Useful? React with 👍 / 👎.
| "multi_output_merge_exact": [ | ||
| { | ||
| "marker": "Create Crafts & Additions rolling did not emit exact wire output", | ||
| "source": "src/createadditionFixture/java/com/swear/autostorage/fixture/createaddition/CreateadditionIntegrationGameTests.java", | ||
| "task": "runCreateadditionGameTestServer" |
There was a problem hiding this comment.
When multi-output handling or duplicate-output merging regresses, this mapped test still passes because the rolling recipe emits one ItemStack whose count is two; it never constructs multiple output entries or exercises a merge. Point multi_output_merge_exact at a test that actually commits and verifies multiple/duplicate outputs instead of letting the twelve-check report claim coverage from this single-output assertion.
AGENTS.md reference: AGENTS.md:L176-L176
Useful? React with 👍 / 👎.
| private static boolean supportsCharging(ChargingRecipe recipe) { | ||
| return recipe.getEnergy() > 0 | ||
| && recipe.getMaxChargeRate() > 0 | ||
| && chargeRate(recipe) > 0 | ||
| && recipe.getIngredients().size() == 1 | ||
| && exact(recipe.getIngredients().getFirst()) | ||
| && !recipe.getResultStack().isEmpty(); |
There was a problem hiding this comment.
When a datapack or another compatible mod version supplies a ChargingRecipe with fluid inputs/results or multiple/probabilistic processing outputs, this predicate still accepts it because it validates only the item ingredient and getResultStack(). chargingPlan then consumes the item and FE while emitting one guaranteed stack, silently discarding the other recipe semantics. Check the inherited fluid and rollable-result collections for the contract's no-fluid, exactly-one-100%-output shape before exposing the recipe.
Useful? React with 👍 / 👎.
| "gui.auto_storage.station.createaddition_rolling_mill": "Rolling Mill", | ||
| "gui.auto_storage.station.createaddition_tesla_coil": "Tesla Coil", |
There was a problem hiding this comment.
These translations become the descriptors' player-facing Processing labels, but both values repeat the installed workstation item names rather than the accepted logical families. Players therefore see Rolling Mill and Tesla Coil on Processing values where the reviewed recipe families are Rolling and Charging; use the shortest localized family/category names in both locale files and leave the concrete machine names to the installed-item tooltips.
AGENTS.md reference: AGENTS.md:L153-L159
Useful? React with 👍 / 👎.
| recipeFamilies.register(rollingId.getPath(), () -> | ||
| RecipeFamilyFactories.deterministicResources( | ||
| RollingRecipe.class, |
There was a problem hiding this comment.
When either processing config is changed from one positive value to another while the server is running, both families remain eligible but continue using the station-work cost captured when their typed contracts were first classified. These registrations use deterministicResources, which caches the typed contract, and expose no dynamic-state token, so Rolling duration and Tesla Coil charge-rate changes do not update craft previews, commits, or the shared Craftable cache until an unrelated recipe-cache reset. Register the config-backed families through the dynamic factory with tokens derived from their live config values.
Useful? React with 👍 / 👎.
Review-controller evidence (PR #74)
Actionable findings (coordinator)
No edit/commit/push/merge by review-controller. |
|
To use Codex here, create an environment for this repo. |
4d813a1 to
44b4c87
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44b4c87dd7
ℹ️ 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".
| || supports(helper, fixtureRecipe("fluid_result_rolling")) | ||
| || supports(helper, fixtureRecipe("fluid_ingredient_rolling")) |
There was a problem hiding this comment.
If any fluid-shape fixture fails to deserialize or is renamed, supports() returns false because byKey() returns empty, so this GameTest still passes without exercising either rejection predicate; unlike the chance fixtures, none of the four fluid fixtures is checked with recipePresent(), and the locked inventory only covers the createaddition namespace rather than auto_storage_createaddition_fixture. Require each fluid recipe to be present before asserting it is unsupported so the optional-mod fixture remains a real behavior assertion.
AGENTS.md reference: AGENTS.md:L173-L173
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 542c735f6b14b240d4b1071a3e3a5fb3e5c877e8. RED confirmed the four JSON fixtures were never loaded: Create rejected them during codec validation because Rolling/Charging allow zero fluid inputs/outputs, so the old supports(null) == false assertions were vacuous. The invalid files are removed. The fixture now constructs exact RollingRecipe and ChargingRecipe instances in memory and requires both upstream validate() rejection and Auto Storage classification rejection; loaded chance fixtures still require recipePresent(). Focused static regression and runCreateadditionGameTestServer (9/9) pass.
|
@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". |
Review-controller evidence (PR #74)
|
542c735 to
fc532d1
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc532d117f
ℹ️ 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".
| private static boolean supportsCharging(ChargingRecipe recipe) { | ||
| return recipe.getEnergy() > 0 | ||
| && recipe.getMaxChargeRate() > 0 | ||
| && chargeRate(recipe) > 0 |
There was a problem hiding this comment.
Fresh evidence in this revision is that the registrations now use dynamicDeterministicResources, while this predicate still makes family membership depend on reloadable configuration. When TESLA_COIL_RECIPE_CHARGE_RATE changes across zero, CraftableRecipeCatalog does not rebuild because it caches membership solely by the RecipeManager snapshot: a recipe indexed while positive remains present and resolving it after the reload can throw when supports() becomes false, while a recipe omitted at zero remains unavailable after the rate becomes positive until recipes reload. The rolling predicate has the same pattern at line 118; keep dynamic-family eligibility stable and fail closed through the re-resolved cost/plan instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b4bef5e. Rolling and Charging eligibility now depends only on invariant recipe shape; reloadable duration/rate is evaluated only by the current cost path, where non-positive values fail closed. Added a static regression that forbids reloadable rate access from both eligibility predicates while requiring the dynamic cost calls. Evidence: focused 3/3, runCreateadditionGameTestServer 9/9, full Python 605/605, git diff --check clean.
| if (craft(context, IRON_ROD) | ||
| || itemCount(context.core(), createadditionItem("iron_rod")) != Long.MAX_VALUE) { |
There was a problem hiding this comment.
If the checked-overflow path rejects only after consuming the iron ingot or station work, this GameTest still passes because it verifies only the failed return and unchanged saturated output. Since the contract maps this marker to checked_overflow_atomic, bundled verification can report exact atomic coverage despite a partial mutation; assert that every seeded input and the station-work balance also remain unchanged.
AGENTS.md reference: AGENTS.md:L176-L176
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b4bef5e. The Long.MAX_VALUE overflow GameTest now asserts the iron input remains 1, the full output remains Long.MAX_VALUE, and accumulated Rolling Mill work remains unchanged. Added a focused static regression. Evidence: runCreateadditionGameTestServer 9/9, full Python 605/605, git diff --check clean.
Independent review-controller evidence
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f578d2f63
ℹ️ 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".
| return Arrays.stream(ingredient.getItems()) | ||
| .filter(stack -> !stack.isEmpty()) | ||
| .map(stack -> stack.copyWithCount(1)) | ||
| .distinct() |
There was a problem hiding this comment.
When a valid simple datapack ingredient repeats the same item alternative, copyWithCount(1) creates distinct ItemStack objects, so this .distinct() does not remove the duplicate; itemKeys() then produces duplicate StorageResourceKey values and TypedRecipeInput throws IllegalArgumentException while the Craftable catalog resolves the recipe, preventing the terminal catalog from building. Deduplicate by canonical resource key while preserving each surviving alternative's corresponding remainder.
Useful? React with 👍 / 👎.
| ResourceLocation missing = createadditionRecipe("rolling/missing_stale_holder"); | ||
| if (craft(context, missing) |
There was a problem hiding this comment.
This test starts with an ID that has never existed, so the first handleRecipeRequest returns before selecting or planning any recipe and never exercises the stale-holder validator used between simulation and commit. A regression that mutates resources after a selected holder is removed or replaced would still let stale_recipe_holder_atomic pass; select a real recipe first, make that holder stale, and then assert the commit is an atomic no-op.
AGENTS.md reference: AGENTS.md:L176-L176
Useful? React with 👍 / 👎.
| if (rate <= 0L) { | ||
| throw new IllegalArgumentException( | ||
| "Create Crafts & Additions charge rate must be positive"); |
There was a problem hiding this comment.
When a live reload makes TESLA_COIL_RECIPE_CHARGE_RATE non-positive, the dynamic token refresh resolves every charging variant again and this exception propagates through RecipeFamily.typedContract and CraftingTerminalMenu.refreshDisplayItems, aborting the menu/server tick instead of merely hiding or disabling those recipes. Fresh evidence beyond the earlier eligibility report is this uncaught dynamic-resolution path; make a non-positive current rate yield no usable variant without throwing. Rolling has the same failure through RecipeFamilyCost.stationWork(rollingDuration()) when its duration becomes non-positive.
Useful? React with 👍 / 👎.
|
|
||
| 2026-08-03 Create Crafts & Additions format-17 recovery：legacy format-7 32個name-shaped candidates經exact artifact `41876c37…e7e9`、clean source `84c7b2ce…4c3a`、normalized NeoForge/Minecraft `2382ea29…eb5f`與完整compile ancestry `migrate-audit`後只剩3個actual `Recipe` classes；`migrate-contract`重開後保留Rolling/Charging accept與Liquid Burning reject，並寫入新audit完整risk set（inherited ProcessingRecipe `generic_ingredients`/`randomness`/`world_mutation`）。Descriptor compile dependencies鎖5個reachable ancestry coordinates，runtime仍是Modrinth createaddition `qPr8V4G2`＋Create `UjX6dr61`；isolated fixture digest `57916d79…d0a3`（110個`createaddition:*`）。不改peer descriptor／不放寬9 MiB gate。Final local gate：createaddition 8/8、bundled verify 12/12 across 5 commands、matrix 3/3（18,503 recipes；Craftable prepare 32.554 ms；shared index 3,711,264 bytes；per-menu 116,493 bytes）、`build`、`runData` no drift、Python 585/585、`git diff --check` clean。高負載下Craftable prepare曾量到52–66 ms；gate維持50 ms，安靜後重跑通過。 | ||
|
|
||
| 2026-08-03 Create Crafts & Additions review-fix gotcha：PR #74 Gemini 指出`itemKeys`對`StorageResourceKey`再`.distinct()`會讓`alternatives`比`representatives`短，`consumedWithRemainder`以index取remainder時可能`IndexOutOfBoundsException`；正確作法是保留與representatives同長的key list，不在`itemKeys`二次去重。同則建議把`chargingWork`的rate clamp成`Math.max(1L, …)`；那會在config reload成非正rate時靜默發明work量，違反No Silent Fallback。正確作法是eligibility已要求正rate，cost路徑對`rate <= 0`直接`IllegalArgumentException` fail-closed，並保留`Math.addExact` overflow檢查。Static regressions鎖住這兩點。 |
There was a problem hiding this comment.
This active note says eligibility already requires a positive rate, but the final implementation deliberately removed the reloadable config rate from supportsCharging and checks only the recipe's positive maxChargeRate; the current rate is evaluated later by the cost path. Leaving the obsolete instruction encourages a future maintainer to restore the exact candidate-membership bug documented earlier in this file, so update the note to distinguish invariant recipe eligibility from the live config check.
AGENTS.md reference: AGENTS.md:L83-L90
Useful? React with 👍 / 👎.
Outcome B accepts deterministic Rolling Mill and Tesla Coil charging from ATM10 createaddition 1.6.0, rejects Liquid Burning and viewer/datagen false candidates, and keeps Create on the bundled compile classpath. via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Keep itemKeys index-aligned with representatives, and fail-closed when chargingWork sees a non-positive live charge rate instead of dividing by zero. via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rebase onto current main isolation and migrate the committed audit/contract to scanner format 17 with exact 1.6.0 artifact, source, and ancestry while preserving Rolling/Charging accept, Liquid Burning reject, and review-fix TDD. via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fail closed on unreviewed Rolling/Charging shapes, invalidate config-backed costs through dynamicDeterministicResources, map remainder and multi-output checks to real assertions, and use JEI family Processing labels. via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Refresh quiet exclusive matrix and verify evidence after additive rebase onto origin/main a1582ad while preserving CEI, runtime transforms, and Createaddition. via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace invalid fluid datapack recipes, which Create rejects before RecipeManager insertion, with exact in-memory Rolling and Charging recipes that prove both upstream validation and Auto Storage classification fail closed. 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>
via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Capture the exclusive matrix and verify evidence after keeping reloadable rates out of eligibility and asserting complete checked-overflow atomicity. via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rebase onto Immersive Engineering main and make stale-holder, dynamic non-positive rate refresh, canonical ingredient dedupe, and eligibility docs fail closed without inventing work or aborting menu ticks. 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>
0f578d2 to
341c647
Compare
|
bugbot run |
|
Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings. |
|
bugbot run |
|
Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive compatibility support for the 'Create Crafts & Additions' mod, enabling deterministic 'Rolling' (Rolling Mill) and 'Charging' (Tesla Coil) recipe families while explicitly excluding liquid burning. The implementation features dynamic cost and plan resolution that fails closed gracefully on invalid configurations, canonical ingredient deduplication, and robust atomic rollback handling. It includes extensive documentation, static regression tests, and nine integration GameTests to verify correct behavior under various conditions like shortages, overflows, and stale recipe holders. No review comments were provided, so I have no feedback to offer on the review itself.
1 participant
Summary
Closes #66.
Evidence-backed Outcome B for Create Crafts & Additions
1.6.0: accept deterministic Rolling and Charging via the public modular SDK; explicitly reject Liquid Burning. Scanner-format-17 recovery on current main isolation; Codex review recovery closes shape fail-closed, live config cost tokens, real remainder/multi-output evidence, JEI family Processing labels, reloadable-rate-independent eligibility, and complete checked-overflow atomic assertions.Recovered onto Immersive Engineering main
d8314fcafter PR #62 merge (additivedocs/overview.md+ static regressions). Addresses Codex review4843430670four actionable findings:IRON_ROD,replaceRecipesremoves it, then asserts complete atomic no-op on STORAGE commit.RecipeFamilypending usesCost.free();resolveVariants/matchturn cost/planIllegalArgumentExceptioninto no usable variant without aborting menu/server tick.StorageResourceKeyputIfAbsentbeforeTypedRecipeInputconstruction.Head
341c647. Local gates: createaddition static 8/8, createaddition GameTests 9/9, Compat Kit verify 12/12 across 5 commands, quiet exclusive matrix 3/3 (21,162 recipes; Craftable prepare 30.682 ms; first/shared p95/warm switch 0.925 / 0.360 / 0.277 ms; storage interaction p95 13.128 ms; shared index 4,113,648 bytes ≈ 3.922 MiB; per-menu 116,952 bytes), Python 610/610,build,runDatawritten 0,git diff --checkclean. 9 MiB／50 ms／128 KiB gates unchanged. No bot review triggered; not merging.Test plan
./gradlew runCreateadditionGameTestServer→ 9/9python3 -m unittest discover scripts→ 610/610./gradlew build+runData(written 0)runCompatibilityMatrixGameTestServer→ 3/3 under 50 ms / 9 MiB / 128 KiBgit diff --checkclean