Repository navigation
feat(streaming): fallback 加 IFC 語意 mapping - #106
Conversation
…-semantic-mapping) `_run_ifcopenshell_openusd_fallback` 產出的 mapping 升級為 IFC class grouped,viewer / coordinator /ui 才能呈現 Semantic ready 真實資料。 變更摘要: - element_mapping items 加 ifc_type / ifc_name / entity_id 欄位 - USD prim path 改為 /World/<IfcClass>/<sanitized_guid>;無 type 進 /World/Unclassified/<sanitized_guid>;per-class Xform 只 Define 一次 - USD-safe identifier sanitization(非 [A-Za-z0-9_] 字元轉 _,開頭非 字母/_前綴 _) - entity_index entries 加 entity_id,與 mapping items 對齊 - quality_metrics 加 semantic_mapping_fidelity / mapping_has_ifc_type / mapping_has_ifc_name - legacy ifc_guid / usd_prim_path 欄位保留,backward compatible - 新增 helpers:_safe_usd_prim_name / _resolve_ifc_class_token / _resolve_guid_token(staticmethod / classmethod) 新增 7 個 unit tests:type/name 帶上、IFC class grouped、Unclassified、 sanitization、quality_metrics 新欄位、entity_id 對齊、backward compat。 不修 HOOPS primary path、不改 convert 簽名、不改 callback outbox、 不引入 production dependency。 OpenSpec change:streaming-server-fallback-semantic-mapping
📝 WalkthroughWalkthroughThis PR enriches the IFC→USDC fallback converter with semantic mapping output by sanitizing IFC-derived identifiers into class-grouped USD prim paths, adding ifc_type and ifc_name fields to mapping items, computing semantic quality metrics, and validating the enhanced pipeline with comprehensive single- and multi-shape tests. ChangesFallback Semantic Mapping Enhancement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR upgrades bim-streaming-server’s IfcOpenShell + OpenUSD fallback conversion outputs so downstream consumers (viewer/coordinator) can rely on richer IFC semantic mapping data (IFC class/name + stable entity alignment) rather than shape-level-only mapping.
Changes:
- Enhance fallback
element_mapping.jsonandentity_index.jsonwithifc_type/ifc_name/entity_id, and add semantic readiness fields toquality_metrics.json. - Change fallback USD prim hierarchy to group meshes under
/World/<IfcClass>/<sanitized_guid>with per-classUsdGeom.Xformdefined once, plus prim-name sanitization helpers. - Add unit tests covering the new mapping fields, prim-path grouping/sanitization, quality metrics additions, and entity-id alignment.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| openspec/changes/streaming-server-fallback-semantic-mapping/tasks.md | Adds an execution checklist for the OpenSpec change. |
| openspec/changes/streaming-server-fallback-semantic-mapping/specs/streaming-ifc-usdc-conversion-authority/spec.md | Spec delta defining new fallback semantic mapping requirements and metrics. |
| openspec/changes/streaming-server-fallback-semantic-mapping/proposal.md | Proposal describing motivation/scope and intended schema/prim-path changes. |
| openspec/changes/streaming-server-fallback-semantic-mapping/design.md | Design detailing schema additions, prim-path strategy, and sanitization rules. |
| bim-streaming-server/tests/test_host_native_conversion_service.py | Adds unit tests validating enriched fallback mapping outputs and metrics. |
| bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/ifc2usdc_powershell_adapter.py | Implements IFC-class-grouped prim paths, enriched mapping/entity docs, and semantic fidelity metrics for fallback conversion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not text: | ||
| return None | ||
| sanitized = "".join( | ||
| ch if ch.isalnum() or ch == "_" else "_" for ch in text | ||
| ) | ||
| if not sanitized: | ||
| return None | ||
| if not (sanitized[0].isalpha() or sanitized[0] == "_"): | ||
| sanitized = "_" + sanitized | ||
| return sanitized |
| - [ ] 1.1 Add unit test `test_fallback_mapping_carries_ifc_type_and_name`:given | ||
| mock IfcOpenShell shape with `guid="GUID_A"`, `name="樓梯1"`, `type="IfcStair"`, | ||
| assert `element_mapping.json` item has `ifc_type="IfcStair"` / | ||
| `ifc_name="樓梯1"` / non-null `entity_id`. | ||
| - [ ] 1.2 Add unit test `test_fallback_prim_paths_are_ifc_class_grouped`:assert | ||
| generated prim path starts with `/World/IfcStair/` for an IfcStair shape. | ||
| - [ ] 1.3 Add unit test `test_fallback_unclassified_grouping`:given shape with | ||
| empty `type`, assert prim path under `/World/Unclassified/`. | ||
| - [ ] 1.4 Add unit test `test_fallback_prim_path_sanitization`:given shape with | ||
| special-char GUID containing `$`, assert sanitized USD-legal identifier. | ||
| - [ ] 1.5 Add unit test `test_fallback_quality_metrics_semantic_fields`:assert | ||
| `quality_metrics.json` contains `semantic_mapping_fidelity == "ifc_class_grouped_with_name"`, | ||
| `mapping_has_ifc_type == True`, `mapping_has_ifc_name == True`. | ||
| - [ ] 1.6 Add unit test `test_fallback_entity_id_alignment`:assert every | ||
| `items[i].entity_id` in `element_mapping.json` appears exactly once in | ||
| `entity_index.json` `entities[].entity_id` with same `ifc_guid` / | ||
| `usd_prim_path`. | ||
| - [ ] 1.7 Add unit test `test_fallback_mapping_backward_compat_keys`:assert | ||
| legacy `ifc_guid` + `usd_prim_path` keys still present on every item. | ||
| - [ ] 1.8 Run `python -m pytest bim-streaming-server/tests/test_host_native_conversion_service.py -k fallback -v` | ||
| and verify all new tests FAIL with expected reasons (missing fields / | ||
| wrong prim paths). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c7777520f
ℹ️ 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".
| ch if ch.isalnum() or ch == "_" else "_" for ch in text | ||
| ) |
There was a problem hiding this comment.
Enforce ASCII-only USD token sanitization
The new _safe_usd_prim_name implementation uses ch.isalnum(), which treats many non-ASCII Unicode characters as valid and therefore preserves them instead of replacing them with _. That breaks this change's own [A-Za-z0-9_] sanitization contract and can emit non-ASCII prim path segments for IFC class/GUID inputs containing localized characters, which may fail downstream consumers that validate or assume ASCII-safe paths. Please switch to an explicit ASCII whitelist check (or regex) when building sanitized tokens.
Useful? React with 👍 / 👎.
…hape test
Review 回饋(HIGH):
1. `_run_ifcopenshell_openusd_fallback` 原本 collision detection 用
`shape_count` 作後綴只後綴一次。`_safe_usd_prim_name` 會把不同特殊字元
GUID(`abc$` / `abc!` / `abc-`)sanitize 成同一 token `abc_`,真實 IFC
含上千 entity 時 sanitized clash 必然發生,UsdGeom.Mesh.Define 對既有
prim 是 reapply 不 raise,會 silently overwrite 前一個 shape 的 mesh
attr。改為「下一個未使用的 __N 後綴」while-loop 確保 prim path 在 stage
內唯一。
2. 既有 7 個 unit test 都是 single-shape,沒覆蓋 multi-shape invariants。
補 `_run_fallback_with_multiple_shapes` helper 與兩個新 test:
- `test_fallback_sanitized_clash_does_not_overwrite_prim`:三個不同
原始 GUID(`abc$`/`abc!`/`abc-`)但 sanitize 同 token,assert 三個
prim path unique + 都 starts with `/World/IfcWall/abc_`
- `test_fallback_entity_id_one_to_one_with_entity_index_multi_shape`:
三個不同 IfcClass shape,assert entity_id set 與 entity_index 對齊、
每筆 ifc_guid + usd_prim_path 一致
verify:
- python -m pytest tests/test_host_native_conversion_service.py → 34 passed
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/ifc2usdc_powershell_adapter.py`:
- Around line 646-662: The _safe_usd_prim_name function currently uses
ch.isalnum() which allows non-ASCII characters; update it to only accept ASCII
letters and digits (A-Z, a-z, 0-9) and underscore so output matches USD regex
`[A-Za-z_][A-Za-z0-9_]*`. Change the character filter in _safe_usd_prim_name to
test membership against ASCII sets (e.g., string.ascii_letters and
string.digits) or an ASCII regex, and ensure the first character check enforces
ASCII letter or underscore; keep the same None fallback behavior when input or
sanitized result is empty.
In `@openspec/changes/streaming-server-fallback-semantic-mapping/proposal.md`:
- Around line 1-2: The document contains English headings and narrative (e.g.,
the "## Why" heading and other English paragraphs at the noted spots) but must
default to Traditional Chinese; update those headings and all general
descriptive text to 繁體中文 while preserving necessary technical tokens (API paths,
field names, code snippets, and technical terms) in their original language, and
ensure the same change is applied to the other English fragments referenced (the
blocks around the noted locations).
In
`@openspec/changes/streaming-server-fallback-semantic-mapping/specs/streaming-ifc-usdc-conversion-authority/spec.md`:
- Around line 9-23: 將本規格文件中英文敘述改為繁體中文(保留必要英文關鍵字與技術名詞不翻譯),例如保留
IfcOpenShell、OpenUSD、element_mapping.json、entity_index.json、ifc_guid、usd_prim_path、Xform、IFC
class 等;請修改「Requirement: Fallback converter emits IFC-semantic
mapping」段落及同一檔案中其他相應描述段落(目前以英文的地方)為繁體中文敘述,同時確保 OpenSpec 必要的 parser 標頭、API
與欄位名稱維持原文英文不變,以免破壞解析或向下相容性。
In `@openspec/changes/streaming-server-fallback-semantic-mapping/tasks.md`:
- Around line 57-67: Update the "Verify" checklist in tasks.md to include the
conversion authority test command required by the streaming server spec: add a
new checklist item under the Verify section (e.g., as 3.6) that runs "python -m
pytest tests/test_conversion_authority_api.py -q". Edit the Verify block in
openspec/changes/streaming-server-fallback-semantic-mapping/tasks.md so the new
line matches the existing style (checkbox format and backticks) and ensure the
list order/numbering remains consistent with the other items.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c4115d5-4738-42a3-9c05-47308662750c
📒 Files selected for processing (6)
bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/ifc2usdc_powershell_adapter.pybim-streaming-server/tests/test_host_native_conversion_service.pyopenspec/changes/streaming-server-fallback-semantic-mapping/design.mdopenspec/changes/streaming-server-fallback-semantic-mapping/proposal.mdopenspec/changes/streaming-server-fallback-semantic-mapping/specs/streaming-ifc-usdc-conversion-authority/spec.mdopenspec/changes/streaming-server-fallback-semantic-mapping/tasks.md
| @staticmethod | ||
| def _safe_usd_prim_name(text: str) -> str | None: | ||
| """Sanitize an arbitrary string into a USD-legal prim name segment. | ||
|
|
||
| USD prim names must match `[A-Za-z_][A-Za-z0-9_]*`. Returns ``None`` | ||
| for empty input (callers fall back to a deterministic placeholder). | ||
| """ | ||
| if not text: | ||
| return None | ||
| sanitized = "".join( | ||
| ch if ch.isalnum() or ch == "_" else "_" for ch in text | ||
| ) | ||
| if not sanitized: | ||
| return None | ||
| if not (sanitized[0].isalpha() or sanitized[0] == "_"): | ||
| sanitized = "_" + sanitized | ||
| return sanitized |
There was a problem hiding this comment.
ch.isalnum() allows non-ASCII alphanumeric characters, producing invalid USD prim names.
Python's str.isalnum() returns True for Unicode alphanumeric characters (e.g., '牆'.isalnum() → True, 'é'.isalnum() → True). IFC files from international projects may contain non-ASCII type/name strings. These characters will pass through unsanitized but are not valid in USD prim names ([A-Za-z0-9_] is ASCII-only), potentially causing USD runtime errors.
🐛 Proposed fix to restrict to ASCII alphanumeric
`@staticmethod`
def _safe_usd_prim_name(text: str) -> str | None:
"""Sanitize an arbitrary string into a USD-legal prim name segment.
USD prim names must match `[A-Za-z_][A-Za-z0-9_]*`. Returns ``None``
for empty input (callers fall back to a deterministic placeholder).
"""
if not text:
return None
sanitized = "".join(
- ch if ch.isalnum() or ch == "_" else "_" for ch in text
+ ch if (ch.isascii() and ch.isalnum()) or ch == "_" else "_" for ch in text
)
if not sanitized:
return None
if not (sanitized[0].isalpha() or sanitized[0] == "_"):
sanitized = "_" + sanitized
return sanitized🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/ifc2usdc_powershell_adapter.py`
around lines 646 - 662, The _safe_usd_prim_name function currently uses
ch.isalnum() which allows non-ASCII characters; update it to only accept ASCII
letters and digits (A-Z, a-z, 0-9) and underscore so output matches USD regex
`[A-Za-z_][A-Za-z0-9_]*`. Change the character filter in _safe_usd_prim_name to
test membership against ASCII sets (e.g., string.ascii_letters and
string.digits) or an ASCII regex, and ensure the first character check enforces
ASCII letter or underscore; keep the same None fallback behavior when input or
sanitized result is empty.
| ## Why | ||
|
|
There was a problem hiding this comment.
OpenSpec 內容語言預設未遵循繁體中文規範。
本檔多個段落標題與敘述以英文為主,超出允許保留原文的例外範圍(API path/欄位名等)。建議將一般敘述與標題改為繁體中文,僅保留必要技術詞原文。
As per coding guidelines, "openspec/**/*.md: OpenSpec artifacts must use Traditional Chinese (繁體中文) by default; ... must remain in original language."
Also applies to: 17-18, 40-41, 56-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openspec/changes/streaming-server-fallback-semantic-mapping/proposal.md`
around lines 1 - 2, The document contains English headings and narrative (e.g.,
the "## Why" heading and other English paragraphs at the noted spots) but must
default to Traditional Chinese; update those headings and all general
descriptive text to 繁體中文 while preserving necessary technical tokens (API paths,
field names, code snippets, and technical terms) in their original language, and
ensure the same change is applied to the other English fragments referenced (the
blocks around the noted locations).
| ### Requirement: Fallback converter emits IFC-semantic mapping | ||
|
|
||
| `bim-streaming-server` 的 `IfcOpenShell + OpenUSD` fallback converter SHALL produce | ||
| `element_mapping.json` items that carry the originating IFC entity type and name | ||
| (when available from IfcOpenShell), align each mapping item with one entity in | ||
| `entity_index.json` via a shared `entity_id`, and structure the USD prim hierarchy | ||
| so each mesh prim is grouped under an `Xform` named after its IFC class. The | ||
| fallback SHALL also publish three quality-metric fields that downstream consumers | ||
| (coordinator `/ui`, viewer) can read to determine semantic readiness without | ||
| having to re-parse the mapping items themselves. | ||
|
|
||
| The new fields and structure SHALL be additive to the existing schema so existing | ||
| consumers that only know the legacy `ifc_guid` + `usd_prim_path` shape continue | ||
| to work without modification. | ||
|
|
There was a problem hiding this comment.
Spec 內文語言請改為繁體中文為主(保留必要英文關鍵字即可)。
目前需求與情境敘述大多為英文,與 OpenSpec「預設繁中」規範不一致。建議將描述性文字改為繁中,保留 parser 必要標頭與 API/欄位等例外原文。
As per coding guidelines, "openspec/**/*.md: OpenSpec artifacts must use Traditional Chinese (繁體中文) by default; ... OpenSpec parser required headers ... must remain in original language."
Also applies to: 24-96, 99-139
🧰 Tools
🪛 LanguageTool
[style] ~21-~21: Consider using “who” when you are referring to people instead of objects.
Context: ...e existing schema so existing consumers that only know the legacy ifc_guid + `usd_...
(THAT_WHO)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@openspec/changes/streaming-server-fallback-semantic-mapping/specs/streaming-ifc-usdc-conversion-authority/spec.md`
around lines 9 - 23, 將本規格文件中英文敘述改為繁體中文(保留必要英文關鍵字與技術名詞不翻譯),例如保留
IfcOpenShell、OpenUSD、element_mapping.json、entity_index.json、ifc_guid、usd_prim_path、Xform、IFC
class 等;請修改「Requirement: Fallback converter emits IFC-semantic
mapping」段落及同一檔案中其他相應描述段落(目前以英文的地方)為繁體中文敘述,同時確保 OpenSpec 必要的 parser 標頭、API
與欄位名稱維持原文英文不變,以免破壞解析或向下相容性。
| ## 3. Verify | ||
|
|
||
| - [ ] 3.1 Run `python -m pytest bim-streaming-server/tests/test_host_native_conversion_service.py -v` | ||
| and verify ALL tests pass (new + existing). | ||
| - [ ] 3.2 Run `python -m pytest bim-streaming-server/tests -q` | ||
| and verify no regression in adjacent suites. | ||
| - [ ] 3.3 Run `python -m pytest tests -p no:cacheprovider` for repo-root contracts | ||
| and fakes;no regression. | ||
| - [ ] 3.4 Run `openspec validate streaming-server-fallback-semantic-mapping --strict` | ||
| (use the `openspec` CLI under `~/AppData/Roaming/npm/openspec`). | ||
| - [ ] 3.5 Run `openspec validate --specs --strict`. |
There was a problem hiding this comment.
驗證清單缺少規範要求的 conversion authority 測試命令。
請在 Verify 區段加入 python -m pytest tests/test_conversion_authority_api.py -q,以符合 streaming server 測試規範。
As per coding guidelines, "bim-streaming-server/tests/**/*.py: Run streaming server conversion authority tests with: python -m pytest tests/test_conversion_authority_api.py -q."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openspec/changes/streaming-server-fallback-semantic-mapping/tasks.md` around
lines 57 - 67, Update the "Verify" checklist in tasks.md to include the
conversion authority test command required by the streaming server spec: add a
new checklist item under the Verify section (e.g., as 3.6) that runs "python -m
pytest tests/test_conversion_authority_api.py -q". Edit the Verify block in
openspec/changes/streaming-server-fallback-semantic-mapping/tasks.md so the new
line matches the existing style (checkbox format and backticks) and ensure the
list order/numbering remains consistent with the other items.
…nge (#110) 4 個 OpenSpec change(#106 / #107 / #108 / #109)merged 後依 AGENTS.md §1.6 完成 sync/archive + roadmap 對齊: archive folders(全部 2026-05-25 archived): - openspec/changes/archive/2026-05-25-streaming-server-fallback-semantic-mapping/ - openspec/changes/archive/2026-05-25-coordinator-serial-conversion-dispatch-queue/ - openspec/changes/archive/2026-05-25-viewer-edge-bim-server-console/ - openspec/changes/archive/2026-05-25-coordinator-ui-tri-ready-and-queue/ openspec/specs/ MODIFIED(無新 capability,皆於既有 spec ADD/MODIFY/REMOVE): - streaming-ifc-usdc-conversion-authority(C1:+1 ADD / ~1 MODIFY) - local-coordinator-ifc-ready-intake-boundary(C4:+1 ADD) - session-first-review-viewer(C2:+2 ADD / ~3 MODIFY / -2 REMOVE) - demo-fast-mvp-orchestration(C3:+3 ADD / ~1 MODIFY) openspec/specs/ 仍 26 個 capability。 openspec validate --specs --strict → 26 passed / 0 failed。 roadmap 同步: - header 加 2026-05-25 fast MVP Edge BIM Data Server Console 4 個 change archive note - §1.4 OpenSpec 已歸檔 change 溯源表加 4 個新 row HTML view 本輪不自動更新(287KB 手寫),follow-up 由 documentation skill 處理。 Verification(依 §1.6,不標 §1.3 runtime passed):streaming pytest 34 + repo-root pytest 9 + coordinator vitest 183 + viewer build/verify 3 scripts + openspec validate 26/0 全 pass。Chrome E2E 屬 follow-up (需 GPU/Kit live runtime,本輪 session 不跑)。
Summary
OpenSpec change:
streaming-server-fallback-semantic-mapping。_run_ifcopenshell_openusd_fallback產出的 mapping 升級為 IFC class grouped,讓 viewer / coordinator/ui能呈現 Semantic ready 真實資料來源。HOOPS A3D primary path 仍 vendor-side 不支援,本 change 不處理。Changes
element_mapping.jsonitems 加ifc_type/ifc_name/entity_id欄位(legacyifc_guid/usd_prim_path保留)/World/<IfcClass>/<sanitized_guid>;無 type 進/World/Unclassified/<sanitized_guid>;per-IFC-classUsdGeom.Xform只 Define 一次[A-Za-z0-9_]字元轉_,開頭非字母/_前綴_,空字串落Shape_NNNNNN(GUID)/Unclassified(class)entity_index.jsonentries 加entity_id,與 mapping items 對齊quality_metrics.json加semantic_mapping_fidelity="ifc_class_grouped_with_name"/mapping_has_ifc_type/mapping_has_ifc_name_safe_usd_prim_name(staticmethod)/_resolve_ifc_class_token/_resolve_guid_token(classmethod)Scope guard
convert對外簽名IfcRelAggregates等 BIM hierarchyTest plan
test_fallback_mapping_carries_ifc_type_and_name/_prim_paths_are_ifc_class_grouped/_unclassified_grouping/_prim_path_sanitization/_quality_metrics_semantic_fields/_entity_id_alignment/_mapping_backward_compat_keys)cd bim-streaming-server && python -m pytest tests -q→ 47 passedpython -m pytest tests -p no:cacheprovider -q(repo root contracts/fakes)→ 9 passedopenspec validate streaming-server-fallback-semantic-mapping --strict→ validopenspec validate --specs --strict→ 26 passed / 0 failedgit diff --cached --check→ cleanArchive gate(post-merge)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation