Skip to content

refactor(engine): migrated AttributeMapper to FlowExpr#2131

Draft
asrcpq wants to merge 15 commits into
mainfrom
refactor/engine/flowexpr-attribute-mapper
Draft

refactor(engine): migrated AttributeMapper to FlowExpr#2131
asrcpq wants to merge 15 commits into
mainfrom
refactor/engine/flowexpr-attribute-mapper

Conversation

@asrcpq
Copy link
Copy Markdown
Contributor

@asrcpq asrcpq commented Jun 3, 2026

Overview

This PR continues #2030, #2087, #2109, #2122 to migrate AttributeMapper to FlowExpr.

Changes

  • migrate AttributeMapper mapper fields to use FlowExpr, update schema.
  • migrate workflows AttributeMapper code to FlowExpr.
  • add containment check for attributes like key in attributes.
  • add string join method like str.join(list).

Notes

  • all direct attribute access are migrated to use valueAttribute which has same null fallback logic.
  • in solar-radiation workflow, invalid AMeDAS CSV (missing column) will fail workflow instead of producing null.

@github-actions github-actions Bot added the engine label Jun 3, 2026
@asrcpq asrcpq force-pushed the refactor/engine/flowexpr-attribute-mapper branch 2 times, most recently from e20ef5f to 52fb882 Compare June 3, 2026 08:54
Copy link
Copy Markdown
Contributor Author

@asrcpq asrcpq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated safety review of unsafe AttributeMapper expr migrations (check.py flagged 26 items). Each inline comment is one item to verify/resolve.

}
expr:
type: flowExpr
value: attributes["path"].split("/")[-2]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[safety review] AttributeMapperGmlPathAndFilename — attributes["path"].split("/")[-2]
Old rhai fell back to udxDirs ?? "" when len < 2. PLATEAU GML paths are always structured as .../udxDir/file.gml with ≥3 segments so [-2] always resolves. The flat 10-cons.yml counterpart already uses this same expression. Safe.

type: flowExpr
value: |
area = attributes["totalRoofArea"];
if area > 0.0 { attributes["totalSolarRadiation"] / area } else { 0.0 }
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[safety review] BuildingPotentialMapper — area = attributes["totalRoofArea"] (dropped ?? 0.0)
Old rhai: let area = env.get("__value")["totalRoofArea"] ?? 0.0. Features arrive from BuildingAreaMerger:merged — only matched features reach here; unmatched go to unmerged (unwired). totalRoofArea is set by BuildingAreaAggregator (StatisticsCalculator sum, always numeric). The ?? 0.0 was defensive dead code. Safe.

d = item.get("uro:depth", 0.0);
if d > max_d { max_d = d }
}
max_d
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[safety review] BuildingAttrExtractor (tsunamiHeight) — same pattern as floodDepth loop above. TsunamiRiskAttribute defaults to [], uro:depth defaults to 0.0. Safe.

env.get("__value").parentTag ?? env.get("__value").featureType
expr:
type: flowExpr
value: attributes.get("parentTag", attributes["featureType"])
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parentTag fallback — potential error — rhai: env.get("__value").parentTag ?? env.get("__value").featureType

value: attributes.get("parentTag", attributes["featureType"])

The default expression attributes["featureType"] is always evaluated regardless. If featureType is also absent it throws, while rhai ?? would return (). Should be attributes.get("parentTag", attributes.get("featureType", "")).

@asrcpq asrcpq force-pushed the refactor/engine/flowexpr-attribute-mapper branch from ba8545c to 37aadfa Compare June 4, 2026 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant