Skip to content

串接 host-native 轉檔與本機 E2E - #75

Merged
monkey1sai merged 1 commit into
mainfrom
codex/openspec/introduce-host-native-conversion-authority-service
May 20, 2026
Merged

monkey1sai merged 1 commit into
mainfrom
codex/openspec/introduce-host-native-conversion-authority-service

Conversation

@monkey1sai

@monkey1sai monkey1sai commented May 20, 2026 •

Copy link
Copy Markdown
Owner

摘要

  • 讓 B 方案 host-native conversion request 不再依賴已退役 worker IDs,保留 IFC-ready intake 的現行邊界。
  • 修正 host-native conversion 的本機預設路徑、PowerShell executable 選擇,以及 coordinator 對退役 8003 conversion API 的 fallback。
  • 修正 web viewer 串流啟動問題:未設定 accessToken 時不啟用 NVIDIA auth,DataChannel 改傳 object,並記錄 LAN IP WebRTC E2E 證據。

驗證

  • python -m pytest tests/test_conversion_authority_api.py tests/test_host_native_conversion_service.py -q:22 passed。
  • npm test(bim-review-coordinator):147 passed。
  • npm run build(bim-review-coordinator):passed。
  • npm run build(web-viewer-sample):passed,僅 Vite large chunk warning。
  • openspec validate introduce-host-native-conversion-authority-service --strict:passed。
  • git diff --check:passed,僅 Windows LF/CRLF 提示。
  • 本機 E2E:使用 storage/許良宇圖書館建築_2026.ifc 完成 external IFC-ready intake -> host-native IFC to USDC -> coordinator review session -> web viewer WebRTC 觀看;Playwright evidence 已記錄於 docs/verification/2026-05-19-introduce-host-native-conversion-authority-service.md。

已知風險 / 注意

  • 目前 real IFC conversion 可開啟且有 renderable prims,但 mapping coverage 仍是 warn:mapped_count=0、coverage_ratio=0.0。
  • WebRTC 在此主機需要 LAN IP endpoint;127.0.0.1 signaling/media 會讓 StreamSDK media candidate 失敗。
  • GitNexus detect-changes 已在 commit 前執行,但 linked worktree 下仍回 No changes detected,所以本 PR 以 git diff、測試、build 與 E2E evidence 補足 scope 驗證。
  • 未修改 .env 或任何 secret 檔;secret scan 只命中 token/accessToken 欄位名稱與既有 dev placeholder。

Summary by CodeRabbit

  • New Features

    • Added support for specifying a public IP address during streaming server startup
    • Enabled optional authentication token support for streaming viewer connections
  • Improvements

    • Enhanced PowerShell executable detection to automatically prefer modern PowerShell when available
    • Improved stream initialization and message handling reliability
  • Documentation

    • Updated verification document with latest test coverage and runtime validation results

Review Change Stack

- 讓 B 方案 conversion request 不再要求退役 worker IDs

- 修正 host-native artifact root、pwsh 選擇與 coordinator conversion API fallback

- 修正 viewer auth/DataChannel 傳訊並補上本機 E2E evidence
Copilot AI review requested due to automatic review settings May 20, 2026 02:49
@coderabbitai

coderabbitai Bot commented May 20, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR consolidates configuration precedence handling, makes conversion job identifiers optional, adds intelligent PowerShell executable selection, normalizes service storage paths, introduces runtime public IP resolution for WebRTC streaming, and updates viewer-side authentication and message passing. The verification document reflects updated build/test results and runtime evidence.

Changes

Host-Native Conversion Service Enhancement

Layer / File(s) Summary
Conversion API base configuration normalization
bim-review-coordinator/src/config.ts, bim-review-coordinator/tests/config.test.ts
Introduces centralized DEFAULT_STREAMING_CONVERSION_API_BASE constant and retired-base set, with conversionApiBaseFromEnv() helper that applies environment precedence (STREAMING_CONVERSION_API_BASE over CONVERSION_API_BASE) and remaps legacy bases back to the new default. loadConfig() updated to use this helper. Test suite verifies precedence and legacy-base normalization.
Optional conversion job identifiers
bim-streaming-server/source/extensions/.../conversion_authority.py, bim-streaming-server/tests/test_conversion_authority_api.py
create_conversion_job now treats export_job_id and source_rvt_artifact_id as optional, storing None when absent/empty rather than validating empty strings. New _safe_optional_id() helper normalizes optional values. Test verifies B-scheme API requests without worker IDs are accepted and return None for those fields.
PowerShell executable selection
bim-streaming-server/source/extensions/.../ifc2usdc_powershell_adapter.py, bim-streaming-server/tests/test_host_native_conversion_service.py
Adds _default_powershell_exe() helper that prefers modern pwsh via shutil.which() and falls back to powershell.exe. adapter_from_env uses this helper when STREAMING_CONVERSION_POWERSHELL_EXE is unset. Tests verify preference for pwsh when available and override behavior when env var is explicitly set.
Service storage path defaults
bim-streaming-server/source/extensions/.../host_native_conversion_service.py, bim-streaming-server/tests/test_host_native_conversion_service.py
Simplifies default cache path from repo_root/bim-streaming-server/_cache/host-native-conversion to repo_root/_cache/host-native-conversion. Test assertion updated to verify new path structure.
WebRTC public IP configuration
bim-streaming-server/scripts/start-streaming-server.ps1
Adds PublicIp parameter (defaults to "auto"), introduces Resolve-PublicIp function that auto-detects non-loopback IPv4 interface addresses or returns provided value, and passes resolved IP to Omniverse streaming extension alongside streamType=webrtc, signalPort, and streamPort. Preflight and startup logging updated to display resolved IP.
Viewer streaming initialization and messaging
web-viewer-sample/src/AppStream.tsx, web-viewer-sample/src/Window.tsx
AppStream now derives stream server from signalingserver prop, conditions authenticate on accessToken presence, and includes accessToken in config only when provided. Window._sendStreamMessage passes objects directly instead of JSON-stringifying. _onStreamStarted clears timeout earlier, updates loading UI text, and defers _pollForKitReady() to setState callback.
Build and runtime verification evidence
docs/verification/2026-05-19-introduce-host-native-conversion-authority-service.md
Updates TDD evidence to report 22 pytest passed (from 19), splits coordinator verification into npm test (147 passed) and npm run build (tsc), adds web-viewer-sample build success note. Runtime evidence upgraded from "blocked" to "passed" with concrete metrics, local E2E viewing details refreshed with observed ports, LAN IP/WebRTC routing, verified URLs, and Playwright facts. Known Risks section updated with mapping coverage status.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • monkey1sai/AI-BIM-governance#73: The core host-native conversion service and adapter implementation that this PR refines with configuration normalization, optional worker IDs, improved PowerShell selection, and path structure adjustments.

Poem

🐰 Hops through the configs, one base at a time,
PowerShell picks the modern sublime,
Worker IDs fade—optional now,
WebRTC streams with a public-IP bow,
Viewer authentication flows so fine!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is written in Chinese and translates to approximately 'Connect host-native conversion and local E2E'. It accurately describes the main objective: integrating host-native conversion with end-to-end local testing, which is the core change across multiple files (config fixes, PowerShell adapter, web viewer adjustments, and verification documentation).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/openspec/introduce-host-native-conversion-authority-service

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

此 PR 針對「host-native 轉檔 + 本機 E2E」串接做整體修補:讓 B 方案 IFC-ready intake 能以 host-native conversion service(49101)完成轉檔並被 coordinator / viewer 消費,同時修正 viewer WebRTC 啟動與 DataChannel 訊息格式、以及 Kit streaming launcher 對 LAN IP(publicIp)的支援,並補齊本機驗證證據文件。

Changes:

  • Web viewer:DataChannel 改送 object(非 JSON 字串)、未提供 accessToken 時不啟用 NVIDIA auth,並改善 stream started 後的 UI/等待狀態處理。
  • bim-streaming-server:host-native conversion 的預設 service_root 路徑修正、PowerShell executable 選擇(prefers pwsh)、B-scheme request 允許 retired worker IDs 欄位為 optional。
  • bim-review-coordinator:忽略 retired http://127.0.0.1:8003 / localhost:8003 conversion API default,改採 streaming conversion API base(49101)並補測試。

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
web-viewer-sample/src/Window.tsx DataChannel 訊息改送 object;stream started 後 UI 狀態/timeout 清理與 Kit ready polling 流程調整。
web-viewer-sample/src/AppStream.tsx 未提供 accessToken 時不啟用 authenticate,並在有 token 時才帶入 accessToken。
docs/verification/2026-05-19-introduce-host-native-conversion-authority-service.md 更新 TDD/build 與本機 E2E(LAN IP / WebRTC)證據與結果。
bim-streaming-server/tests/test_host_native_conversion_service.py 補強 host-native config 預設路徑與 PowerShell exe 選擇行為測試。
bim-streaming-server/tests/test_conversion_authority_api.py 新增 B 方案 request 不再要求 retired worker IDs 欄位的 regression test。
bim-streaming-server/source/extensions/…/ifc2usdc_powershell_adapter.py 預設 PowerShell 優先用 pwsh;env adapter 改用該預設並維持顯式覆蓋優先。
bim-streaming-server/source/extensions/…/host_native_conversion_service.py 修正 host-native service_root 預設落點(避免多一層 bim-streaming-server/)。
bim-streaming-server/source/extensions/…/conversion_authority.py export_job_id / source_rvt_artifact_id 改為 optional(None/空字串允許)。
bim-streaming-server/scripts/start-streaming-server.ps1 新增 -PublicIp auto 自動解析 LAN IPv4,並注入 Kit livestream primaryStream publicIp 設定。
bim-review-coordinator/tests/config.test.ts 新增 conversion API base 的 retired 8003 fallback/precedence 測試。
bim-review-coordinator/src/config.ts 統一 conversionApiBase 決策:優先 STREAMING_CONVERSION_API_BASE,忽略 retired 8003 預設。

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@web-viewer-sample/src/Window.tsx`:
- Line 297: Revert the change that passes the object directly to
AppStream.sendMessage by serializing the payload: update the call to
AppStream.sendMessage to pass JSON.stringify(message) instead of message so the
sendMessage API (AppStream.sendMessage) receives a string as required by the
WebRTC/data-channel contract; locate the invocation where
AppStream.sendMessage(message) is used and wrap the message with
JSON.stringify(message).
🪄 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: 477e2112-d558-4aaa-b176-b5fdcce31d23

📥 Commits

Reviewing files that changed from the base of the PR and between 6569e57 and 46a3390.

📒 Files selected for processing (11)
  • bim-review-coordinator/src/config.ts
  • bim-review-coordinator/tests/config.test.ts
  • bim-streaming-server/scripts/start-streaming-server.ps1
  • bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/conversion_authority.py
  • bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/host_native_conversion_service.py
  • bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/ifc2usdc_powershell_adapter.py
  • bim-streaming-server/tests/test_conversion_authority_api.py
  • bim-streaming-server/tests/test_host_native_conversion_service.py
  • docs/verification/2026-05-19-introduce-host-native-conversion-authority-service.md
  • web-viewer-sample/src/AppStream.tsx
  • web-viewer-sample/src/Window.tsx

return;
}
AppStream.sendMessage(JSON.stringify(message));
AppStream.sendMessage(message);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

@nvidia/omniverse-webrtc-streaming-library sendMessage API format

💡 Result:

The sendMessage API in the @nvidia/omniverse-webrtc-streaming-library is used to send custom messages from a web client to a streaming Omniverse Kit application [1][2][3]. Format Requirements: The sendMessage method expects a string argument [3]. To ensure compatibility with the Kit application—specifically when using extensions like omni.kit.livestream.messaging—it is standard practice to pass a JSON-serialized string [3][4]. Implementation Example: To send a message, define your message object, convert it to a JSON string, and pass it to the AppStreamer.sendMessage method [3][4]: // Define the message object const message = { event_type: "your_event_type", payload: { key: "value" } }; // Send the JSON-stringified message AppStreamer.sendMessage(JSON.stringify(message)); Key Considerations: 1. Data Channel: Messages are transmitted over the WebRTC data channel established by the AppStreamer [4]. 2. Handling Incoming Messages: To receive messages on the client side, you must register a handler (typically named onCustomEvent) within the stream configuration passed to AppStreamer.setup [5][3]. 3. Kit-Side Integration: On the Omniverse Kit application side, these messages are typically processed by extensions such as omni.kit.livestream.messaging, which expects structured JSON data [6][3][4]. Ensure your message structure aligns with the expectations of the specific Kit extension you are communicating with [3].

Citations:


Revert the sendMessage argument back to JSON.stringify(message).

Line 297 changes from AppStream.sendMessage(JSON.stringify(message)) to AppStream.sendMessage(message). The sendMessage API expects a string argument, and all NVIDIA documentation and examples explicitly require passing a JSON-serialized string. Passing the object directly breaks the messaging contract and will fail at the library level when the WebRTC data channel attempts to transmit the message.

🤖 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 `@web-viewer-sample/src/Window.tsx` at line 297, Revert the change that passes
the object directly to AppStream.sendMessage by serializing the payload: update
the call to AppStream.sendMessage to pass JSON.stringify(message) instead of
message so the sendMessage API (AppStream.sendMessage) receives a string as
required by the WebRTC/data-channel contract; locate the invocation where
AppStream.sendMessage(message) is used and wrap the message with
JSON.stringify(message).

@monkey1sai
monkey1sai merged commit 2bc1e82 into main May 20, 2026
5 checks passed
@monkey1sai
monkey1sai deleted the codex/openspec/introduce-host-native-conversion-authority-service branch May 20, 2026 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants