Team-EnderIO/EnderIO

Fork 0

fix: Port fix for loading large conduit networks from 1.20.1 #1007

Merged

Rover656 merged 3 commits from fix/conduit-recursion-21 into dev/1.21.1

2025-03-16 20:29:28 +00:00

Rover656 commented

2025-02-19 19:43:01 +00:00

(Migrated from github.com)

Description

Ports the fix from GH-1005 to 1.21.1.

Checklist

My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you).
I have made corresponding changes to the documentation.
My changes are ready for review from a contributor.

Summary by CodeRabbit

Bug Fixes
- Improved reliability by filtering out incomplete or invalid network data during the loading process, minimising potential disruptions.
Refactor
- Streamlined the integration mechanism for linking network components, optimising performance and enhancing error reporting.

# Description Ports the fix from GH-1005 to 1.21.1.  # Checklist - [x] My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you). - [ ] I have made corresponding changes to the documentation. - [x] My changes are ready for review from a contributor.   ## Summary by CodeRabbit - **Bug Fixes** - Improved reliability by filtering out incomplete or invalid network data during the loading process, minimising potential disruptions. - **Refactor** - Streamlined the integration mechanism for linking network components, optimising performance and enhancing error reporting.

CrazyPants commented

2025-02-22 13:03:14 +00:00

(Migrated from github.com)

Don't really understand whats going on in with code, but I tested the changes and everything seems to be working without issue.

❤️ 1

Rover656 commented

2025-03-12 17:53:01 +00:00

(Migrated from github.com)

@coderabbitai review

coderabbitai[bot] commented

2025-03-12 17:53:07 +00:00

(Migrated from github.com)

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

<details> <summary>:white_check_mark: Actions performed</summary> Review triggered. > Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused. </details>

coderabbitai[bot] commented

2025-03-12 17:53:10 +00:00

(Migrated from github.com)

Walkthrough

The changes update the graph deserialization process in the conduit saved data. In the deserializeGraphs method, parameterised logging is used for missing conduits, and a new check skips processing graphs that contain no objects. The approach for linking graph objects is revised to create a main graph from the first object and link neighbours accordingly. Additionally, a critical error handling mechanism now throws an IllegalStateException if the graph remains null after processing. The redundant merge method has been removed to streamline the logic.

Changes

File	Changes Summary
enderio-conduits/.../ConduitSavedData.java	Updated `deserializeGraphs`: improved parameterised logging for missing conduits, introduced check to skip graphs without objects, restructured graph object linking, added error handling with `IllegalStateException`, and removed the obsolete `merge` method.

Sequence Diagram(s)

sequenceDiagram
    participant CD as ConduitSavedData
    participant G1 as GraphObject (Initial)
    participant G2 as GraphObject (Neighbour)
    participant Log as Logger

    CD->>CD: Start deserializeGraphs
    CD->>CD: Check for empty graph objects
    alt Graph is empty
        CD->>Log: Log missing conduits (parameterised)
        CD-->>CD: Skip processing graph
    else Graph contains objects
        CD->>G1: Create graph from first object
        CD->>G2: Identify & link neighbour graph objects
        CD->>CD: Validate constructed graph
        alt Graph is null
            CD->>Log: Log critical error
            CD-->>CD: Throw IllegalStateException
        end
    end

Poem

I'm a rabbit with a hop in my code,
Leaping over bugs on a smooth, new road.
Graphs now link with a cheerful band,
Invalid data skipped as planned.
With each clever hop and twist,
My code feels light – oh, what a bliss!
Hippity-hop, improvements in command!

Tip

⚡🧪 Multi-step agentic review comment chat (experimental)

We're introducing multi-step agentic chat in review comments. This experimental feature enhances review discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments.
- To enable this feature, set early_access to true under in the settings.

✨ Finishing Touches

📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

## Walkthrough The changes update the graph deserialization process in the conduit saved data. In the `deserializeGraphs` method, parameterised logging is used for missing conduits, and a new check skips processing graphs that contain no objects. The approach for linking graph objects is revised to create a main graph from the first object and link neighbours accordingly. Additionally, a critical error handling mechanism now throws an `IllegalStateException` if the graph remains null after processing. The redundant `merge` method has been removed to streamline the logic. ## Changes | File | Changes Summary | |---------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | enderio-conduits/.../ConduitSavedData.java | Updated `deserializeGraphs`: improved parameterised logging for missing conduits, introduced check to skip graphs without objects, restructured graph object linking, added error handling with `IllegalStateException`, and removed the obsolete `merge` method. | ## Sequence Diagram(s) ```mermaid sequenceDiagram participant CD as ConduitSavedData participant G1 as GraphObject (Initial) participant G2 as GraphObject (Neighbour) participant Log as Logger CD->>CD: Start deserializeGraphs CD->>CD: Check for empty graph objects alt Graph is empty CD->>Log: Log missing conduits (parameterised) CD-->>CD: Skip processing graph else Graph contains objects CD->>G1: Create graph from first object CD->>G2: Identify & link neighbour graph objects CD->>CD: Validate constructed graph alt Graph is null CD->>Log: Log critical error CD-->>CD: Throw IllegalStateException end end ``` ## Poem > I'm a rabbit with a hop in my code, > Leaping over bugs on a smooth, new road. > Graphs now link with a cheerful band, > Invalid data skipped as planned. > With each clever hop and twist, > My code feels light – oh, what a bliss! > Hippity-hop, improvements in command!   > [!TIP] > <details> > <summary>⚡🧪 Multi-step agentic review comment chat (experimental)</summary> > > - We're introducing multi-step agentic chat in review comments. This experimental feature enhances review discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments. > - To enable this feature, set early_access to true under in the settings. > > </details>      <details> <summary>✨ Finishing Touches</summary> - [ ]  📝 Generate Docstrings </details>   --- <details> <summary>🪧 Tips</summary> ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=Team-EnderIO/EnderIO&utm_content=1007): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit <commit_id>, please review it.` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (Invoked using PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai generate docstrings` to [generate docstrings](https://docs.coderabbit.ai/finishing-touches/docstrings) for this PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://docs.coderabbit.ai) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements. </details>

coderabbitai[bot] (Migrated from github.com) reviewed

2025-03-12 17:55:30 +00:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 0

🧹 Nitpick comments (3)

enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitSavedData.java (3)
97-103: Add logging for empty graph objects.
You have a TODO comment suggesting a warning. An explicit log statement (e.g. LOGGER.warn("Empty graph encountered. Skipping...")) could help diagnose potential data inconsistencies.
 if (graphObjectsTag.isEmpty()) {
-    // TODO: Warning or something?
+    LOGGER.warn("Encountered an empty graph. Skipping the loaded data...");
     continue;
 }
128-143: Consider avoiding the manual cast when connecting neighbours.
The explicit cast to GraphObject<ConduitGraphContext> could be replaced by stronger typing in your data structures or a helper method. Minimising unchecked casts keeps the code safer and clearer.

145-159: Validate additional debugging information in the error message.
This block correctly throws an exception if the graph is null, but consider including the conduit type or other context in the error message to aid debugging.
 LOGGER.error(
-        "Graph is null after loading a network. Please report this issue to Ender IO, loading cannot continue.");
+        "Graph is null after loading a network for conduit '{}'. Please report this issue to Ender IO, loading cannot continue.",
+        conduitKey // or a relevant identifier
 );

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4061eacf82 and f79b54f85a.

📒 Files selected for processing (1)

enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitSavedData.java (3 hunks)

🔇 Additional comments (3)

enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitSavedData.java (3)

85-85: Log level usage appears appropriate.
This warning is correct, as it indicates skipping a conduit type that is genuinely missing.

160-165: Properly overwriting existing context data.
Overwriting previously constructed context is acceptable here, though ensure this behaviour aligns with the rest of the system’s expectations (e.g. partial merges vs. full overwrite). Currently, there are no issues detected with the approach.

124-127:
✅ Verification successful

Ensure getFirst() is valid for non-empty lists.
Since you skip empty lists at lines 97–103, calling graphObjects.getFirst() is logically safe. However, confirm that the custom getFirst() method (if not a standard library extension) handles non-collection types gracefully or that the library used is well-tested.

🏁 Script executed:
#!/bin/bash
# Attempt to locate any custom definitions of getFirst() in the codebase
fd -t f --exec rg "getFirst"
Length of output: 3581
Review Verification: getFirst() Usage Confirmed

After verifying the codebase, no custom implementation of getFirst() was identified. All invocations—including the one at lines 124–127 in ConduitSavedData.java—appear to rely on a standard or well-tested library method. Given that empty lists are precluded (as confirmed at lines 97–103), the use of graphObjects.getFirst() is safe.

There is no indication that getFirst() requires special handling for non-collection types, so the current implementation is sound. Should there be any future ambiguity, consider referencing the library documentation to confirm that its behaviour remains as expected.

**Actionable comments posted: 0** <details> <summary>🧹 Nitpick comments (3)</summary><blockquote> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitSavedData.java (3)</summary><blockquote> `97-103`: **Add logging for empty graph objects.** You have a TODO comment suggesting a warning. An explicit log statement (e.g. `LOGGER.warn("Empty graph encountered. Skipping...")`) could help diagnose potential data inconsistencies. ```diff if (graphObjectsTag.isEmpty()) { - // TODO: Warning or something? + LOGGER.warn("Encountered an empty graph. Skipping the loaded data..."); continue; } ``` --- `128-143`: **Consider avoiding the manual cast when connecting neighbours.** The explicit cast to `GraphObject<ConduitGraphContext>` could be replaced by stronger typing in your data structures or a helper method. Minimising unchecked casts keeps the code safer and clearer. --- `145-159`: **Validate additional debugging information in the error message.** This block correctly throws an exception if the graph is null, but consider including the conduit type or other context in the error message to aid debugging. ```diff LOGGER.error( - "Graph is null after loading a network. Please report this issue to Ender IO, loading cannot continue."); + "Graph is null after loading a network for conduit '{}'. Please report this issue to Ender IO, loading cannot continue.", + conduitKey // or a relevant identifier ); ``` </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 4061eacf82f99c8ea68e20099cd5f8878c2d460a and f79b54f85a0cf5db093a36b3def5d595b7d8c574. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitSavedData.java` (3 hunks) </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitSavedData.java (3)</summary> `85-85`: **Log level usage appears appropriate.** This warning is correct, as it indicates skipping a conduit type that is genuinely missing. --- `160-165`: **Properly overwriting existing context data.** Overwriting previously constructed context is acceptable here, though ensure this behaviour aligns with the rest of the system’s expectations (e.g. partial merges vs. full overwrite). Currently, there are no issues detected with the approach. --- `124-127`: <details> <summary>✅ Verification successful</summary> **Ensure `getFirst()` is valid for non-empty lists.** Since you skip empty lists at lines 97–103, calling `graphObjects.getFirst()` is logically safe. However, confirm that the custom `getFirst()` method (if not a standard library extension) handles non-collection types gracefully or that the library used is well-tested. --- 🏁 Script executed: ```shell #!/bin/bash # Attempt to locate any custom definitions of getFirst() in the codebase fd -t f --exec rg "getFirst" ``` Length of output: 3581 --- </details> **Review Verification: `getFirst()` Usage Confirmed** After verifying the codebase, no custom implementation of `getFirst()` was identified. All invocations—including the one at lines 124–127 in *ConduitSavedData.java*—appear to rely on a standard or well-tested library method. Given that empty lists are precluded (as confirmed at lines 97–103), the use of `graphObjects.getFirst()` is safe. There is no indication that `getFirst()` requires special handling for non-collection types, so the current implementation is sound. Should there be any future ambiguity, consider referencing the library documentation to confirm that its behaviour remains as expected. </details> </blockquote></details> </details>

Rover656 commented

2025-03-16 20:29:22 +00:00

(Migrated from github.com)

Ignoring the nitpicks for now and merging :)