feat(engine): update dependencies to version 0.7.5 and add calamine support #798
Conversation
WalkthroughThis pull request introduces updates to the project's dependency management and adds a new module for processing object lists. The changes primarily involve updating several Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
✅ Deploy Preview for reearth-flow canceled.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
engine/runtime/action-plateau-processor/src/object_list.rs (2)
10-13: Consider broadening error variants for better clarity.
TheErrorenum currently only has one variant (Parse). While this is functional, employing multiple error variants (e.g., for I/O issues, missing columns, and parsing logic) can improve clarity and debuggability.
82-85: Naming consistency consideration.
requiredvstargetmight benefit from naming that clearly indicates the intent (e.g.,required_pathsvsoptional_paths). This could improve code readability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
engine/Cargo.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (4)
engine/Cargo.toml(2 hunks)engine/runtime/action-plateau-processor/Cargo.toml(1 hunks)engine/runtime/action-plateau-processor/src/lib.rs(1 hunks)engine/runtime/action-plateau-processor/src/object_list.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- engine/runtime/action-plateau-processor/src/lib.rs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules - reearth-flow
- GitHub Check: Header rules - reearth-flow
- GitHub Check: Pages changed - reearth-flow
- GitHub Check: ci-engine / ci
🔇 Additional comments (5)
engine/runtime/action-plateau-processor/src/object_list.rs (2)
25-58: Validate column length or default to avoid index pitfalls.
When extracting fields fromcolumns, the code usesunwrap_or("".to_string()). Although safe from panics, it can mask data anomalies ifcolumnsis unexpectedly short. Consider adding optional logging or validations to handle incomplete rows more explicitly.
72-90: ObjectList construction logic looks sound.
Mapping eachRecordintoObjectListensures a structured approach for grouping feature types. The choice to storerequiredvstargetin separate arrays seems appropriate for the design.engine/runtime/action-plateau-processor/Cargo.toml (1)
29-30: Confirm necessity of workspace flags and duplication in dev-dependencies.
bytesandcalamineare added as workspace dependencies. Ensure that their versions or features match across the workspace, and confirm thatbytesis indeed needed in both[dependencies]and[dev-dependencies].engine/Cargo.toml (2)
67-70: Version updates look correct.
Upgrading thenusamai-*dependencies fromv0.7.4tov0.7.5aligns with the PR objective of updating to version 0.7.5. Confirm successful compilation and any associated changes in their APIs.
85-85: Confirm pinning strategy for calamine.
Addingcalamine = "0.26.1"is fine. Verify whether this aligns with the workspace usage inaction-plateau-processor/Cargo.toml, ensuring no version conflicts.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
engine/runtime/action-plateau-processor/src/object_list.rs (2)
9-13: Consider adding aWorksheetNotFoundvariant.
Since the code attempts to handle missing worksheets at line 223, it may be cleaner to introduce a dedicated variant likeWorksheetNotFoundin theErrorenum. This will improve error handling clarity.#[derive(Error, Debug)] pub(crate) enum Error { #[error("Parse error: {0}")] Parse(String), + #[error("Worksheet not found")] + WorksheetNotFound, }
186-198: Incorporate flexible category checks.
should_process_rowonly checks for "主題" or "関連役割." If future categories are added, you may need to extend this filter or make it configurable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
engine/runtime/action-plateau-processor/src/object_list.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci-engine / ci
🔇 Additional comments (6)
engine/runtime/action-plateau-processor/src/object_list.rs (6)
1-2: Imports look good.
No concerns regarding these standard library imports for collections and I/O.
38-71: Validate bounds when accessing columns.
The indexing (columns.get(...)) approach is safe, but you might add logging or a warning if the row is missing columns 0–13, since they're crucial for building theRecord. This will ensure better diagnostic messages if the Excel file is malformed.
111-179: Keep up the good usage of stateful parsing.
TheAttributeStatestruct is well-organized and helps avoid scattering mutable logic. Good job encapsulating attribute transitions.
181-184: Confirm large file performance.
open_workbookreads the entire workbook in memory. For extremely large Excel files, consider a streaming approach if calamine supports it.
200-214: Potential prefix expansions.
expand_row_for_special_prefixcurrently expands only the first column if it starts with"fld/". This is fine, but remain aware of potential changes in naming conventions.
219-266: Break down the parse logic into smaller functions.
Theparsemethod is large and centralizes I/O and row processing logic. Splitting the loop into smaller composable functions could improve readability and testability.
Overview
What I've done
What I haven't done
How I tested
Screenshot
Which point I want you to review particularly
Memo
Summary by CodeRabbit
Dependencies
nusamaipackage dependencies fromv0.7.4tov0.7.5calaminelibrary version0.26.1bytesdependency for development purposesNew Features
object_listmodule