Repository navigation
feat(worker): 實作 IFC 轉 USDC 真實轉檔品質閘 - #24
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (21)
📝 WalkthroughWalkthroughThis PR upgrades the ChangesReal Conversion Implementation and Testing
Contract and OpenSpec Documentation
🎯 4 (Complex) | ⏱️ ~60 minutes
✨ 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 _worker’s IFC→USDC conversion path from placeholder outputs to a real conversion adapter with hard quality gates (notably USDC openability), and propagates converter identity + quality metrics through the worker result payload, tests, and documentation/spec evidence so downstream components don’t treat placeholder artifacts as review-ready.
Changes:
- Add an IFC→USDC converter adapter boundary (
ifcopenshell+usd-core) that emits realmodel.usdc, indices, mapping, and quality metrics. - Enforce “real artifact” readiness by failing conversions that are non-openable, placeholder-like, mock mapping, or missing required outputs (and keep artifact groups non-ready on failure).
- Update smoke script, tests, OpenSpec artifacts, and contract/verification docs to reflect the new quality gate and evidence requirements.
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/smoke-worker-review-request.ps1 | Switch smoke flow to dev IFC sources and assert USDC openability quality gate before creating a review session. |
| README.md | Clarify worker smoke prerequisites (real IFC in storage/, real converter prerequisites). |
| openspec/changes/worker-real-conversion-quality/tasks.md | Add completed OpenSpec task checklist for the change. |
| openspec/changes/worker-real-conversion-quality/specs/worker-artifact-pipeline/spec.md | Define requirements for real conversion artifacts, mapping schema, and quality gates. |
| openspec/changes/worker-real-conversion-quality/specs/runtime-verification-evidence/spec.md | Define evidence requirements distinguishing API success vs real geometry success. |
| openspec/changes/worker-real-conversion-quality/README.md | Introduce the OpenSpec change scope. |
| openspec/changes/worker-real-conversion-quality/proposal.md | Record motivation, scope, and non-goals for the real conversion quality gate. |
| openspec/changes/worker-real-conversion-quality/design.md | Document design decisions (adapter boundary, readiness semantics, measure-first coverage, one-to-many mapping). |
| openspec/changes/worker-real-conversion-quality/.openspec.yaml | Register the OpenSpec change metadata. |
| docs/verification/evidence/2026-05-11-worker-real-conversion-quality/single-kit-runtime-summary.json | Store single Kit/browser runtime capture summary for the verification evidence set. |
| docs/verification/2026-05-11-worker-real-conversion-quality.md | Add detailed verification record for the real conversion quality gate and runtime evidence. |
| docs/contracts/worker-api.md | Extend worker API contract with converter + quality metrics fields and failure semantics. |
| docs/contracts/streaming-datachannel-events.md | Update mapping/prim-path guidance to reflect one-to-many mapping and alias field behavior. |
| docs/contracts/local-dev-runbook.md | Update runbook to reflect new smoke prerequisites and placeholder rejection. |
| _worker/tests/test_worker_store.py | Add store-level tests for quality metrics, converter unavailability, placeholder rejection, and openability gate handling. |
| _worker/tests/test_worker_converters.py | Add unit tests for converter mapping rules (don’t count missing/unknown GUIDs as real coverage). |
| _worker/tests/test_worker_api.py | Update API tests for new payload fields, failure behavior, and opt-in real converter smoke. |
| _worker/README.md | Update worker README to describe real converter prerequisites, gates, and mapping schema. |
| _worker/app/store.py | Replace placeholder conversion with converter adapter execution + result validation + readiness gating and new payload fields. |
| _worker/app/main.py | Allow injecting a converter into the app; avoid posting callbacks for failed conversions. |
| _worker/app/converters.py | Implement the IfcOpenShellUsdConverter adapter producing USDC + indices + mapping + quality metrics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| raise ConversionAdapterError("Generated element_mapping.json contains fake mapping entries.") | ||
|
|
||
| def _looks_like_placeholder(self, path: Path) -> bool: | ||
| content = path.read_bytes()[:4096].lower() |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6a99e6475
ℹ️ 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".
| [string] $ModelVersionId = "version_demo_001", | ||
| [string] $UserId = "dev_user_001", | ||
| [string] $DevSourceId = "", | ||
| [int] $TimeoutSeconds = 30 |
There was a problem hiding this comment.
Now that this script selects a real dev IFC source and runs the IfcOpenShell/OpenUSD converter, the 30s default is too short for the documented validation path: the evidence added in docs/verification/2026-05-11-worker-real-conversion-quality.md records duration_seconds: 189.15. Running scripts/smoke-worker-review-request.ps1 without overriding -TimeoutSeconds will therefore fail while the conversion is still legitimately running, making the advertised smoke check unreliable for real IFC inputs; please raise the default or derive it from source size.
Useful? React with 👍 / 👎.
| if job.get("status") != "succeeded" or not job.get("result"): | ||
| return |
There was a problem hiding this comment.
When the new real converter fails under the default auto_complete background path (for example missing ifcopenshell/usd-core or an invalid IFC), this early return prevents posting the failed result to _bim-control. _bim-control already maps non-succeeded conversion results to blocked_conversion, so skipping the callback leaves the data authority with no conversion-result/artifact-group state and review-session requests cannot surface the failure; post failed results as well, or add an explicit failure callback.
Useful? React with 👍 / 👎.
| adapter_result = self.converter.convert( | ||
| source_path=Path(self.settings.objects_root) / source["object_key"], | ||
| output_dir=root_path, |
There was a problem hiding this comment.
For artifacts created with source_url or signed_upload_url, _content_bytes stores only a JSON upload_reference object, but this new converter path always passes the persisted object as the IFC source file. In deployments that use signed upload references instead of content_base64, the real converter will try to parse that JSON stub as IFC and the job can never succeed; materialize the referenced bytes before this call or reject reference-only artifacts before queuing conversion.
Useful? React with 👍 / 👎.
| } | ||
| warnings = list(job.get("warnings") or []) | ||
| warnings.append(f"{code}: {message}") | ||
| return self._update_job(job["conversion_job_id"], status="failed", stage=stage, result=result, warnings=warnings) |
There was a problem hiding this comment.
If a conversion is rerun for a source whose artifact group is already ready, and the new real converter fails (for example after prerequisites are removed or the source is reprocessed with bad input), this failure path only updates the job record and leaves the existing artifact-group index untouched. The group therefore continues to advertise the previous derived model.usdc as ready even though the latest conversion result is ready=false, which violates the new failure semantics and can route review sessions to stale artifacts; update the group to a failed/blocked or missing-derived state here.
Useful? React with 👍 / 👎.
CH-3 收 stage host 設定化 + 死碼清理 + 文件對齊 3 風險(#12 stage allowed-hosts 設定化+warn / #24 移除三處退役 8005 死碼 / #14 SYSTEM_DESIGN 改 as-built)。spec delta: one-click-deploy-hybrid ADD 1(stage allowed-hosts 設定化+空值告警+預設不含退役 port);#14/README/fixture tasks-only。openspec validate --strict 通過。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…_DESIGN as-built (#12 #24 #14) CH-3 apply: - #12 stage allowed-hosts 設定化+空值 warn: stage_loading.py 空值加 carb.log_warn; start-streaming-server.ps1 加 -AllowedStageHosts param + 三分支(param→env→Write-Warning+default),去除靜默 fallback - #24 移除三處退役 _worker :8005 死碼: stage_loading.py 內建預設 / start-streaming-server.ps1 / deploy.ps1 Resolve-AllowedStageHosts / README,只留 49101(explore 確認 100% 可安全移除) - #14 SYSTEM_DESIGN.md 改寫 as-built: 新增 As-built 段 + §5-13 前瞻逐段標 DEFERRED + 補實際描述(對 codebase 核實); §3/§9 target sizing 保留加註(roadmap 引用) 驗證: pytest stage_loading 3 passed(+2); root pytest 65; test-deploy-dryrun ALL PASSED(含 #24 Test 10); stage-loading-contract passed。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
變更摘要
實作
_worker的 IFC -> USDC 真實轉檔品質閘，讓 worker 產出的model.usdc、index 與element_mapping.json來自實際 IFC 幾何資料，而不是 placeholder 產物。同步補齊 OpenSpec task、API contract、驗證紀錄與 single Kit/browser 截圖證據。修改原因
目前 worker conversion flow 需要可審查的真實 USDC artifact，並且必須在 artifact group ready 前證明輸出可由 USD stage 開啟、mapping/coverage 可量測、失敗或缺少 converter prerequisite 時不會發布 ready artifact。此變更把轉檔品質閘落在
_worker邊界，避免 downstream coordinator/viewer/Kit 把 placeholder artifact 當成正式模型。主要變更
_worker/app/converters.py，以 IfcOpenShell + OpenUSD (usd-core) 建立 IFC -> USDC adapter boundary。_worker/app/store.py、_worker/app/main.py，在 conversion result 中回傳converter、quality_metrics、real artifact URLs，並在失敗或 non-openable USDC 時阻止 artifact group ready。_worker/tests/test_worker_store.py、_worker/tests/test_worker_api.py，覆蓋 converter success、prerequisite missing、invalid output、placeholder rejection、one-to-many mapping shape 與 opt-in real smoke skip。docs/contracts/worker-api.md、docs/verification/2026-05-11-worker-real-conversion-quality.md、OpenSpec specs/tasks，記錄品質閘、coverage measure-first policy 與驗證證據。docs/verification/evidence/2026-05-11-worker-real-conversion-quality/，包含 full-page screenshot、viewport screenshot 與 runtime summary JSON。驗證方式
cd _worker && python -m pytest tests\test_worker_store.py -q --basetemp .pytest_tmp：45 passed。cd _worker && $env:PYTHONPATH=(Resolve-Path .test-deps).Path; python -m pytest tests\test_worker_api.py -q --basetemp .pytest_tmp：32 passed, 1 skipped。skipped 項目是WORKER_RUN_REAL_USDC_SMOKE=1opt-in real converter smoke。openspec validate worker-real-conversion-quality --strict：通過。openspec instructions apply --change worker-real-conversion-quality --json：32/32 tasks complete。gitnexus detect_changes(scope=all)：risk level low，affected processes 0。scripts/smoke-worker-review-request.ps1 -TimeoutSeconds 900：通過，real IFC fixture89394282bytes，coverage_ratio=0.950556913882097，conversion_job_id=conv_20260511034506_f88ee0fd，session_id=review_session_001a59d345ce。readyState=4、videoWidth=1920、videoHeight=1080、srcObject=true、bodyHasDataChannelReply=true、pixelStats.nonBlack=14385，截圖位於docs/verification/evidence/2026-05-11-worker-real-conversion-quality/single-kit-review_session_001a59d345ce-real-conversion.png。風險與影響
_workerconversion 行為由 placeholder 改成真實 adapter；缺少ifcopenshell或usd-core時 conversion job 會失敗且不發布 ready artifact group。ifcopenshell 0.8.5package classifier 為LGPLv3+；usd-core 26.5package metadata 為LicenseRef-TOST-1.0。正式部署前需確認授權與散佈政策。node_modules與 ignored 89 MB IFC fixture；CI 環境未必能重跑 single Kit/browser 截圖。converter/quality_metrics。回滾方式
若需撤回，revert 本 PR 的 commits，重點是還原
_worker/app/converters.py、_worker/app/store.py、_worker/app/main.py、測試與對應 docs/OpenSpec/evidence。回滾後 worker 會回到原本 placeholder conversion 行為。後續建議
ifcopenshell/usd-core的安裝方式與 license review 結果落到正式部署文件。Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests