Repository navigation
[codex] 正規化 architecture rework OpenSpec change - #52
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (26)
📝 WalkthroughWalkthroughThis pull request introduces a complete OpenSpec architecture redesign package that redefines service responsibilities, conversion job ownership, and integration contracts across the BIM platform. It shifts IFC→USDC conversion authority from ChangesArchitecture Rework 2026-05-14: Service Boundary & Conversion Authority Redesign
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 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 normalizes the architecture-rework-2026-05-14 OpenSpec change into the structure the OpenSpec CLI recognizes. Previously the content lived inside a wrapper directory (architecture-rework-2026-05-14-openspec/openspec/changes/...), which caused the CLI to display no-tasks and prevented validation by change id. The PR adds the standard OpenSpec artifacts (proposal.md, design.md, tasks.md, 13 spec deltas) plus supporting package/contract drafts, all documentation-only — no runtime code is touched.
Changes:
- Add the canonical
proposal.md,design.md, andtasks.mdunderopenspec/changes/architecture-rework-2026-05-14/. - Add 13 ADDED / MODIFIED spec deltas defining the B-scheme architecture (streaming-server as IFC→USDC authority, worker as RVT→IFC bridge, platform as deployment boundary, etc.).
- Preserve the original package README, decision alignment, manifest, architecture notes, and contract drafts under
package/anddocs/.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| openspec/changes/architecture-rework-2026-05-14/proposal.md | Why/What/Scope/Success criteria for the rework. |
| openspec/changes/architecture-rework-2026-05-14/design.md | Detailed design: decisions, target roles, contracts, migration phases, risks. |
| openspec/changes/architecture-rework-2026-05-14/tasks.md | Numbered task checklist with branch/baseline items marked done. |
| specs/bim-control-revit-intake-facade/spec.md | New capability: fake Revit/RVT intake in _bim-control. |
| specs/worker-rvt-ifc-bridge/spec.md | New capability: _worker as RVT→IFC bridge. |
| specs/worker-artifact-pipeline/spec.md | Modify: split bridge from streaming-owned conversion. |
| specs/streaming-ifc-usdc-conversion-authority/spec.md | New capability: streaming-server owns IFC→USDC. |
| specs/streaming-usd-stage-composition/spec.md | New capability: primary/secondary USD layer composition. |
| specs/streaming-multi-layer-payload-loading/spec.md | Modify: openStageRequest payload semantics. |
| specs/session-first-review-viewer/spec.md | Modify: viewer displays streaming-owned status. |
| specs/review-session-request-lifecycle/spec.md | Modify: session references streaming conversion readiness. |
| specs/multi-artifact-kit-routing/spec.md | Modify: primary/secondary composition policy. |
| specs/bim-review-platform-boundary/spec.md | New capability: platform as deployment boundary. |
| specs/conversion-webhook-lifecycle/spec.md | New capability: correlation IDs and idempotent webhooks. |
| specs/demo-runtime-readiness-smoke/spec.md | Modify: B-scheme readiness tiers. |
| specs/documentation-source-of-truth/spec.md | Modify: source-of-truth alignment rules. |
| package/README.md, APPLY_INSTRUCTIONS.md, DECISION_ALIGNMENT.md, PACKAGE_MANIFEST.json | Original package notes preserved. |
| docs/architecture/ARCHITECTURE_ALIGNMENT_NOTES.md | Architecture alignment narrative. |
| docs/contracts/drafts/*.md | Draft JSON/HTTP contracts for the new flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: d276ec848d
ℹ️ 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".
|
|
||
| ## MODIFIED Requirements | ||
|
|
||
| ### Requirement: Worker artifact pipeline separates RVT→IFC bridge from streaming-owned IFC→USDC conversion |
There was a problem hiding this comment.
This requirement is under ## MODIFIED Requirements, but there is no requirement with this title in the existing openspec/specs/worker-artifact-pipeline/spec.md (I checked the current requirement headings). The same pattern appears in the other modified deltas, so strict OpenSpec validation/archive will treat these as modifications to non-existent requirements rather than new requirements, blocking the change even though tasks.md marks validation as complete. Use ADDED for genuinely new requirements or modify the exact existing requirement titles.
Useful? React with 👍 / 👎.
| #### Scenario: Conversion job failure is honest | ||
|
|
||
| - **WHEN** converter execution fails, USDC is missing, USDC cannot be opened, or mapping generation fails past allowed policy | ||
| - **THEN** `bim-streaming-server` marks the job `failed` or `succeeded_with_warnings` only when explicitly allowed |
There was a problem hiding this comment.
This scenario permits succeeded_with_warnings, but the same design's conversion status enum only defines queued, running, succeeded, failed, and cancelled. When implementers wire the status/result contract or tests from these specs, warning conversions will either use an undocumented status or be rejected by enum validation. Please add succeeded_with_warnings to the status enum/contract or model warnings as fields on succeeded.
Useful? React with 👍 / 👎.
| "touches_runtime_code": false, | ||
| "files": [ | ||
| { | ||
| "path": "APPLY_INSTRUCTIONS.md", |
There was a problem hiding this comment.
The manifest's path values are inconsistent: this entry only exists under package/APPLY_INSTRUCTIONS.md, the docs/... entries only exist under the change directory, and the openspec/changes/... entries only resolve from the repo root. Any integrity check or packaging script that reads this manifest with a single base directory will fail to find or hash a subset of the files, so the package cannot be verified reliably.
Useful? React with 👍 / 👎.
|
|
||
| ### D2. Keep live streaming runtime separate from heavy conversion execution | ||
|
|
||
| Although `bim-streaming-server` owns the conversion job, the actual heavy converter SHOULD run in a headless converter app / subprocess / job lane, not inside the live WebRTC viewport runtime thread. |
There was a problem hiding this comment.
The repo-local AGENTS.md says OpenSpec artifacts should default to Traditional Chinese, with only API paths, schema fields, CLI flags, status enums, logs/errors, external product names, and parser-required headings kept in their original language. This design body is English prose, and the same pattern appears across several added specs/contracts, which violates the documentation source-of-truth rule reviewers and local stakeholders rely on. Please translate the prose while leaving contract literals unchanged.
Useful? React with 👍 / 👎.
|
|
||
| ### Modified existing capabilities | ||
|
|
||
| - `worker-artifact-pipeline` |
There was a problem hiding this comment.
This change lists worker-artifact-pipeline as the only worker conversion capability to modify, but repo-wide search shows existing specs such as worker-dev-ifc-source-selection and worker-demo-upload-convert-ui still require _worker to create conversion jobs, return _worker conversion results, and hand off worker-produced model.usdc. After archiving this change, those untouched source-of-truth specs will still contradict the B-scheme rule that _worker is no longer the IFC→USDC authority, so implementers will have conflicting requirements.
Useful? React with 👍 / 👎.
| "force": false, | ||
| "allow_placeholder_ready": false, | ||
| "allow_fake_mapping": false | ||
| } |
There was a problem hiding this comment.
The create-job request in this contract closes without callback_url, while the design's _worker → bim-streaming-server payload includes that field and the webhook lifecycle spec requires _bim-control callback failures to be tracked separately. If implementers build from this contract, the streaming server has no per-job callback target to notify, so conversion_result_ready delivery either has to rely on an undocumented global default or cannot satisfy the callback observability requirements.
Useful? React with 👍 / 👎.
變更內容
architecture-rework-2026-05-14正規化為 OpenSpec CLI 可識別的 change 目錄。proposal.md、design.md、tasks.md與 13 個 delta spec 檔案。package/、docs/。.git、執行 strict validate。為什麼
原本內容被包在
architecture-rework-2026-05-14-openspec/openspec/changes/...，OpenSpec CLI 只能看到外層 wrapper，導致 change 顯示no-tasks並且無法用正確 change id 驗證。這個 PR 把它整理成正式 OpenSpec change 格式，後續才能依 task list 推進 B 方案架構重構。驗證
openspec validate architecture-rework-2026-05-14 --strictopenspec validate --all --strictdetect_changes(scope=staged)：risk levellow，changed symbols0，affected processes0未納入本 PR
AGENTS.mdCLAUDE.mdAI-BIM-governance standalone - 重構.html本 PR 先只處理 OpenSpec change 正規化，不進行 runtime/code implementation。
Summary by CodeRabbit