-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(engine): update workflow execution and adjust orientation condition in surface validator #641
Conversation
…ion in surface validator
WalkthroughThe changes involve modifications to several YAML configuration files and the main function of the example feature transformer. The main function's argument for the workflow file was updated to a new YAML file for quality checks. Additionally, a new quality check workflow was introduced, which includes various parameters and nodes for processing city GML data. The surface validation workflow was also adjusted to change the orientation condition from "clockwise" to "counter_clockwise." Changes
Possibly related PRs
Suggested labels
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.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
engine/runtime/examples/plateau/testdata/workflow/quality-check/07-lsld/workflow.yml (2)
42-75
: Consider optimizing attribute aggregationThe attribute aggregation could be simplified by using a single calculation block for all metrics.
Consider consolidating the calculations:
aggregateAttributes: - newAttribute: Index attributeValue: | env.get("__value").fileIndex.to_string() - newAttribute: Folder - attribute: package + attributeValue: | + env.get("__value").package - newAttribute: Filename attributeValue: | file::extract_filename(env.get("__value").cityGmlPath) - newAttribute: "フィーチャータイプ" - attribute: featureType + attributeValue: | + env.get("__value").featureType
76-191
: Review error handling in surface validationThe surface validation workflow appropriately handles two types of errors:
- Invalid surface orientation
- Invalid surface geometry
However, there's potential for improvement in error reporting.
Consider adding:
- Error severity levels
- Detailed error descriptions
- Suggestions for fixing common issues
engine/runtime/examples/plateau/testdata/graphs/surface_validator_2d.yml (1)
Line range hint
1-447
: LGTM: Well-structured workflow configurationThe overall workflow configuration is well-structured with clear separation of concerns:
- Surface validation steps are logically organized
- Error handling paths are properly defined
- Node connections maintain a clear flow of data
Consider adding documentation comments for each major section of the workflow to improve maintainability. For example:
# Surface Orientation Validation # Validates the orientation of surface boundaries according to CityGML/PLATEAU specifications - id: 27cd7ebf-7d16-4a1c-ac27-02eddb61a7b9 name: FeatureFilterByOrientation ...
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
engine/runtime/examples/plateau/example_feature_transformer.rs
(1 hunks)engine/runtime/examples/plateau/testdata/graphs/surface_validator_2d.yml
(1 hunks)engine/runtime/examples/plateau/testdata/workflow/quality-check/07-lsld/workflow.yml
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- engine/runtime/examples/plateau/example_feature_transformer.rs
🔇 Additional comments (4)
engine/runtime/examples/plateau/testdata/workflow/quality-check/07-lsld/workflow.yml (3)
23-41
: LGTM: Input processing setup is well structured
The input processing chain (FolderAndfilePathReader → FeatureReader → LodSplitterWithDm) is properly configured to handle CityGML data.
193-320
: Verify edge connections in result processing
The result processing section uses FeatureMerger to combine multiple data streams. Ensure all edge connections are properly defined.
#!/bin/bash
# Description: Verify edge connections for result processing
# Expected: All referenced node IDs exist and are properly connected
# Extract and verify node IDs
rg -o 'id: ([a-f0-9-]+)' . | sort | uniq -d
# Check for dangling edges
rg 'from: ([a-f0-9-]+)' . | cut -d: -f2 | sort > /tmp/from_nodes
rg 'id: ([a-f0-9-]+)' . | cut -d: -f2 | sort > /tmp/all_nodes
comm -23 /tmp/from_nodes /tmp/all_nodes
16-19
: Verify surface validator changes
The workflow includes a surface validator component that may have been modified to handle counter-clockwise orientation as mentioned in the PR summary.
✅ Verification successful
Surface validator is correctly configured for counter-clockwise orientation
The surface validator configuration explicitly checks for counter-clockwise orientation in the FeatureFilterByOrientation
node:
conditions:
- expr: |
env.get("__value").outerOrientation != "counter_clockwise"
outputPort: inCorrectOrientation
This validates that surfaces must have counter-clockwise orientation, which aligns with the PR's changes. Features with incorrect orientation are routed to the inCorrectOrientation
port for handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for recent changes to surface validator configuration
# Expected: Changes related to orientation validation from clockwise to counter-clockwise
# Check surface validator changes
rg -p "orientation|clockwise|counter" ../../../graphs/surface_validator_2d.yml
Length of output: 1258
Script:
#!/bin/bash
# Let's try a different approach to find the surface validator configuration
# First, let's locate the exact file
fd "surface_validator_2d.yml" --type f
# Then let's examine its contents
cat $(fd "surface_validator_2d.yml" --type f)
Length of output: 13671
engine/runtime/examples/plateau/testdata/graphs/surface_validator_2d.yml (1)
43-43
: Verify alignment with CityGML/PLATEAU specifications
The orientation condition has been changed from "clockwise" to "counter_clockwise". While the change is technically sound, we should verify that this aligns with the CityGML/PLATEAU specifications for surface orientation.
Let's check the repository for any documentation or specifications about surface orientation:
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
New Features
Bug Fixes