Skip to content

docs(verification): 2026-05-08 review-session spec end-to-end verification - #16

Merged
monkey1sai merged 1 commit into
mainfrom
claude/flamboyant-johnson-e60968
May 8, 2026
Merged

monkey1sai merged 1 commit into
mainfrom
claude/flamboyant-johnson-e60968

Conversation

@monkey1sai

@monkey1sai monkey1sai commented May 8, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • 紀錄 2026-05-08 對 openspec/specs/review-session-request-lifecycle/ 與 openspec/specs/session-first-review-viewer/ 兩個 spec 的端對端驗證結果。
  • 涵蓋 _bim-control pytest(21/21)、bim-review-coordinator vitest(102/102)、web-viewer-sample session-first contract、scripts/smoke-worker-review-request.ps1 端對端串接、雙 Chrome tab multi-user 真實互動。
  • 此 PR 不修改任何 production 程式碼;僅新增 docs/verification/2026-05-08-spec-end-to-end-verification.md。

已驗證

  • _bim-control review-session-request CRUD、必填驗證、artifact readiness、blocked_conversion / queued_for_instance / closing / closed / failed 全狀態機。
  • coordinator session lifecycle、artifact_bindings[]、kit_instance_bindings[]、close → release 分離、idempotent close。
  • viewer 從 session_id / review_request_id bootstrap、blocked / closing / closed gating、runtime command(DataChannel)與 collaboration(Socket.IO)分流。
  • WebRTC signaling + DataChannel + openStageRequest 從 viewer 路由到 Kit。
  • Multi-user Socket.IO joinSession + annotationCreate 經 coordinator 廣播 + 寫入 _bim-control(annotation_id=ann_1778212872)。

仍未驗證 / 限制(hardware / fixture dependent)

  • Kit GPU viewport 實際渲染 USD:smoke 用的 storage/sample.ifc 只含 ISO-10303-21 兩行 header,轉出的 USD 沒可渲染幾何。Kit App 端對端通了(送收 DataChannel),但 viewport 看不到圖。需有效 IFC fixture 才能補。
  • dedicated_instance 多 Kit instance 並行 streaming:本機 kit_profile=local_fixed 只有 1 個 Kit signaling(49100),第二個 viewer 撞 0xC0F22219 GPU busy。需 ≥ 2 個 Kit instance 才能驗 multi-stream routing。
  • 大型 IFC 端對端壓力:smoke fixture 是極小檔案,沒驗 conversion 大檔耗時、記憶體、status=processing 中間態下 viewer UI 行為。
  • Socket.IO 大量併發:本次只實測 2 個瀏覽器 tab;coordinator vitest 涵蓋 broadcast 邏輯,但無多 user 真實壓力測試。
  • bim-streaming-server contract 測試:本次未跑 bim-streaming-server/scripts/tests/test-stage-loading-contract.ps1(streaming pipeline 對 USD 載入的 contract)。

環境提醒(與 spec 無關)

  • 系統 Python 安裝過 starlette 1.0.0,與 _bim-control/requirements.txt 鎖定的 fastapi 0.111.0 / starlette 0.37.2 不相容。需在 worktree 建立獨立 .venv 安裝 pinned 版本後才能跑 pytest。
  • bim-review-coordinator 與 web-viewer-sample 的 node_modules 預設未提交,需先 npm install 才能跑 npm test / npm run test:session-first。

Test plan

  • _bim-control && python -m pytest tests/test_review_session_requests_api.py -v
  • bim-review-coordinator && npm test
  • web-viewer-sample && npm run test:session-first
  • scripts/smoke-worker-review-request.ps1
  • 端對端 lifecycle / close / release(已收乾)
  • 雙 Chrome tab multi-user annotationCreate 廣播與 _bim-control 持久化

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Updated end-to-end verification and specification documentation with comprehensive test coverage details, including automated test results and manual validation checks across application components and workflows.

…verification

涵蓋 review-session-request-lifecycle 與 session-first-review-viewer 兩個 spec 的自動測試、API smoke、雙 Chrome tab multi-user collaboration 實測,並列出 GPU viewport 渲染與 dedicated_instance routing 等 hardware-dependent 仍未驗證的項目。

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 8, 2026 04:08
@coderabbitai

coderabbitai Bot commented May 8, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Adds a comprehensive end-to-end verification report documenting scope, automated test commands across three services, API smoke testing with lifecycle validations, browser multi-user WebRTC and Socket.IO checks, and a checklist separating verified functionality from unverified limitations and follow-up work.

Changes

2026-05-08 End-to-End Verification Report

Layer / File(s) Summary
Report Scope and Overview
docs/verification/2026-05-08-spec-end-to-end-verification.md
Report title and verified object scope (two OpenSpec spec directories) with coverage statement for automated suites, API smoke, browser multi-user flows, and lifecycle transitions.
Automated Test Coverage
docs/verification/2026-05-08-spec-end-to-end-verification.md
Specific commands for _bim-control pytest, bim-review-coordinator vitest, and web-viewer-sample contract tests with environment setup and execution notes.
API Smoke Testing Procedures
docs/verification/2026-05-08-spec-end-to-end-verification.md
API smoke workflow with request/response chains and lifecycle/binding validations: 422 on missing fields, lifecycle-events content, review-session state/binding rewrites, and close endpoint semantics.
Browser Multi-User Validation
docs/verification/2026-05-08-spec-end-to-end-verification.md
WebRTC and DataChannel routing validation, Kit load error attribution, multi-instance routing behavior, Socket.IO collaboration with annotation persistence, and close/release transitions with timestamps.
Verification Summary and Cleanup
docs/verification/2026-05-08-spec-end-to-end-verification.md
Checklist of verified vs unverified functionality (GPU rendering, parallel streaming, IFC performance, high-concurrency Socket.IO), follow-up recommendations, and session cleanup appendix.

Estimated Code Review Effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A verification report hops into view,
Documenting tests both old and new,
API smoke trails, WebRTC flows,
Browser tabs clicking as the pipeline grows—
Five ranges of truth, no code to review! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately summarizes the main change: adding documentation for 2026-05-08 end-to-end verification of review-session specifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/flamboyant-johnson-e60968

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.

@monkey1sai
monkey1sai merged commit 568f4cf into main May 8, 2026
2 of 3 checks passed
@monkey1sai
monkey1sai deleted the claude/flamboyant-johnson-e60968 branch May 8, 2026 04:10

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 595ae5a566

ℹ️ 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".


啟動端口:`_bim-control`(8001)、`_worker`(8005)、`bim-review-coordinator`(8004)、`bim-streaming-server` Kit signaling(49100)、`web-viewer-sample` dev server(5173)。

開兩個 Chrome tab,帶 `?sessionId=review_session_6721d4c09e6d&userId=user_alpha&displayName=Alpha` 與 `?userId=user_bravo&displayName=Bravo`。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include the session id in the second tab URL

The documented reproduction URL for Tab B omits sessionId, so a reader following this verification step will not join review_session_6721d4c09e6d unless they also have an unstated VITE_DEFAULT_SESSION_ID set. I checked web-viewer-sample/src/config/env.ts and Window.tsx: defaultSessionId comes from the query/env, and when it is empty the viewer auto-creates a new review session, which means the participants=2 and cross-tab broadcast evidence below would not be reproduced for the same session.

Useful? React with 👍 / 👎.

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

This PR adds a dated verification report documenting end-to-end validation of two OpenSpec specs (review-session-request-lifecycle and session-first-review-viewer) across _bim-control, bim-review-coordinator, web-viewer-sample, and an E2E smoke script, without changing production code.

Changes:

  • Added a verification report capturing test results (pytest/vitest/contract test) and manual multi-user browser validation.
  • Documented observed lifecycle behavior (close/release separation, idempotent close) and known unverified areas/constraints (GPU viewport, multi-instance, load testing).

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

Comment on lines +14 to +25
| Spec / Requirement | 實作位置 | 測試入口 |
| --- | --- | --- |
| review-session-request-lifecycle Req1(POST /api/review-session-requests + 必填驗證) | `_bim-control/app/main.py:498` | `_bim-control/tests/test_review_session_requests_api.py` |
| Req2(artifact readiness、blocked_conversion) | `_bim-control/app/main.py:290`、`216`、`701` | 同上 |
| Req3(coordinator session 回寫 + queued_for_instance) | `_bim-control/app/main.py:559`、`bim-review-coordinator/src/services/kitPool.ts` | 同上 + `bim-review-coordinator/tests/sessions.test.ts` |
| Req4(lifecycle 顯式狀態) | `_bim-control/app/main.py:568`、`bim-review-coordinator/src/app.ts:273` | 同上 |
| Req5(closing vs instance release 分離) | `bim-review-coordinator/src/app.ts:287`、`src/services/kitPool.ts:86` | `bim-review-coordinator/tests/sessions.test.ts:264, 566` |
| session-first-review-viewer Req1(從 review_request_id / session_id bootstrap) | `web-viewer-sample/src/Window.tsx:355` 起 | `web-viewer-sample/scripts/verify-session-first-contract.mjs` |
| Req2(顯示 artifact / lifecycle / Kit binding readiness) | `web-viewer-sample/src/Window.tsx:126`、`components/ArtifactPanel.tsx` | 同上 |
| Req3(runtime command 走 DataChannel;collaboration 走 coordinator) | `web-viewer-sample/src/clients/streamMessages.ts`、`reviewSocket.ts` | 同上 |
| Req4(multi-artifact 控制) | `web-viewer-sample/src/clients/streamMessages.ts` `buildOpenStageRequest` | 同上 |
| Req5(lifecycle transition 安全) | `web-viewer-sample/src/Window.tsx:229, 297` | 同上 |
Comment on lines +31 to +35
| 套件 | 入口 | 結果 |
| --- | --- | --- |
| `_bim-control` pytest(review session lifecycle 套組) | `cd _bim-control && python -m pytest tests/test_review_session_requests_api.py -v` | 21 / 21 通過 |
| `bim-review-coordinator` vitest(4 檔:sessions / kitpool / sessionstore / dev-console) | `cd bim-review-coordinator && npm test` | 102 / 102 通過 |
| `web-viewer-sample` session-first contract | `cd web-viewer-sample && npm run test:session-first` | 通過 |
| Req2(artifact readiness、blocked_conversion) | `_bim-control/app/main.py:290`、`216`、`701` | 同上 |
| Req3(coordinator session 回寫 + queued_for_instance) | `_bim-control/app/main.py:559`、`bim-review-coordinator/src/services/kitPool.ts` | 同上 + `bim-review-coordinator/tests/sessions.test.ts` |
| Req4(lifecycle 顯式狀態) | `_bim-control/app/main.py:568`、`bim-review-coordinator/src/app.ts:273` | 同上 |
| Req5(closing vs instance release 分離) | `bim-review-coordinator/src/app.ts:287`、`src/services/kitPool.ts:86` | `bim-review-coordinator/tests/sessions.test.ts:264, 566` |

- Kit signaling 49100 連線建立。
- WebRTC SDP offer/answer + ICE 完成(SDP 含 `BUNDLE 0 1 2`、`ice-ufrag`)。
- viewer 透過 DataChannel 送 `openStageRequest`,UI ⑥ 顯示 `openedStageResult`。
monkey1sai added a commit that referenced this pull request Jun 2, 2026
- #27(P2): _pollForKitReady 進入點改 _clearPollForKitReady(),閉合 in-mount 重入孤兒並行 chain(原僅 id=null 不 clearTimeout)
- #28(P2): _resetState 參數化 connectionText,逾時訊息不再被 _resetState 的 connectionText:'' 覆寫
- #32(P2): GFN script 命中既有但全域 GFN 未就緒時補掛 load/error,避免 remount-during-load 仍 ReferenceError(用 //@ts-ignore 不引入 as any)
- design.md #8: 修正捏造的 SDK 引用(實裝 5.17.0/L71/terminate(terminateApp?) 無 _force)
- 補 triReady.test.ts(11 test,含 #16 spectator started+matched→yes);Decision 7 誠實揭露 structLog.test.ts defer

驗證:build + vitest 32 passed(21→32) + struct-log 10 + session-first 全綠。

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
monkey1sai added a commit that referenced this pull request Jun 2, 2026
- #32(A): _initStream 改 globalThis.GFN 讀取,缺失走 onStreamFailed,避免 onload 後 GFN 未建立仍裸變數 ReferenceError(Copilot+Codex)
- #15(B): selectSpectatorBinding 先驗 primaryKitInstanceId 在 bindings 內才用 id 挑,否則退 port-diff,避免 coordinator 資料不一致時誤選 primary(Copilot)
- #16(D): spectator 僅在 viewport_sharing.spectator_ready 時 stageLoadStatus='matched',否則 pending(Codex);連帶 verify 斷言 + spec scenario 精確化
- envHelpers.test 補 null case(CodeRabbit);vitest.config 補分號對齊 .prettierrc semi(Copilot)

驗證:build + vitest 34 passed(32→34) + struct-log 10 + session-first 全綠。
defer(PR 揭露):reconnect await terminate、GFN unmount guard(pre-existing async / gfn 窄窗競態)。

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
monkey1sai added a commit that referenced this pull request Jun 2, 2026
reviewRequestId/BimControlClient endpoint gap(既有,非 #18 引入)、#28 session
teardown、#16 spectator readiness refresh、reconnect await terminate 等 P2
列為 follow-up(pre-existing / 跨 repo / 超 spec scope),本 PR 不擴大 scope。

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
monkey1sai added a commit that referenced this pull request Jun 2, 2026
harden-web-viewer-test-resilience: #17 vitest+jsdom 骨架(34 test) / #27 timer / #8 terminate(false) / #28 poll 上限 / #15 spectator binding / #16 spectator_ready gate / #18 env boundary / #32 GFN 動態載入。雙層 review(opus 5-lens + Copilot/Codex/CodeRabbit)。
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