移除衝突檢討、viewer 改 127.0.0.1 bind (remove-conflict-review-from-fast-mvp predecessor) - #90
Merged
monkey1sai merged 2 commits intoMay 21, 2026
Conversation
…mvp loop brainstorming spec
- 新增 openspec/changes/remove-conflict-review-from-fast-mvp/{proposal,design,tasks}.md
與 specs/review-session-request-lifecycle/spec.md(## REMOVED Requirements 框架)
- 新增 docs/superpowers/specs/2026-05-21-fast-mvp-loop-overall-design.md
涵蓋兩個 change 切分(Approach B)、Section 1/2/3 設計、/goal acceptance condition
僅 design phase 落地,implementation tasks(Change 1 §1-§14)等使用者 review
brainstorming spec 後透過 /goal 啟動;Change 2 OpenSpec change folder 等
predecessor archived 後再建(NoSuccessorWhilePredecessorOpen gate)。
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
remove-conflict-review-from-fast-mvp 完整實作 (predecessor for fast-ifc-link-demo-loop)。
純減法 + 一行 compose port bind + docs/spec 退役 note。
Coordinator:
- 刪 highlight/selection/annotation Socket.IO handlers
- 刪 BimControlClient.getReviewIssues / createAnnotation + ReviewIssue type
- 刪 /api/model-versions/:id/review-bootstrap endpoint + safeIssues helper
- 改 registerReviewNamespace signature(不再吃 bimControlClient)
- dev-console.html step bar 5 步改 3 步、刪 3 張 guided card、刪 raw emit 按鈕
- dev-console.js / inline script 對應函式刪
- tests/dev-console.test.ts + tests/sessions.test.ts 對齊新行為
Viewer:
- 刪 IssuePanel / EventLogPanel components + types/issues(整檔)
- 刪 viewer bimControlClient.getReviewIssues / coordinatorClient.getReviewBootstrap
- 刪 reviewSocket emit{Highlight,Selection,Annotation} / demoDefaults.{demoIssueId,buildDemoHighlightItem}
- Window.tsx 清 reviewIssues state、_onIssueClick、issue/annotation methods、IssuePanel render、DataChannel issue branch
- DemoControlPanel 4 個 issue/annotation props 改 optional(JSX 按鈕等 Change 2 viewer 重做時整段刪)
- verify-session-first-contract 改驗 session-first → stream-config 鏈(review-bootstrap 已退役)
Compose:
- compose.host-kit.yml viewer.ports 改 127.0.0.1:5173:5173(對應 webrtc-1on1 邊界)
Docs:
- AGENTS.md §5.4 / §5.5 / §7.4 加退役 note
- bim-review-coordinator/CLAUDE.md Role 段加退役 note
OpenSpec:
- specs/review-session-request-lifecycle/spec.md ## MODIFIED Coordinator exposes lifecycle event
audit log;加 implementation status note,排除清單 wording 保留作 archive compatibility
Verification (5 級):
- L1 coordinator npm run verify: 11 files / 166 tests passed
- L1 viewer build + test:session-first: passed
- L1 root pytest tests: 9 passed
- L2 openspec validate remove-conflict-review-from-fast-mvp --strict: valid
- L2 openspec validate --specs --strict: 26 passed / 0 failed
- L3 git diff --stat: 23 files / +96 / -625 lines — 影響面 = expected
- L4 docker compose up --build coordinator viewer: 兩 container Up
- L4 netstat: 0.0.0.0:5173 已消失,僅剩 127.0.0.1:5173
- L4 docker exec coordinator /health: status=ok
- L4 docker exec /ui 字串斷言:「步驟 ③ / 3」=true、「標示問題位置/建立審查標註/guidedHighlightIssue」=false
- L5 mcp__claude-in-chrome navigate: permission denied,改以 L4 container 內 fetch 字串斷言代替
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
monkey1sai
deleted the
codex/openspec/remove-conflict-review-from-fast-mvp
branch
May 21, 2026 11:47
Contributor
|
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 (27)
📝 WalkthroughWalkthroughThis pull request removes the "conflict review" feature set (issue highlighting, selection, annotation, and related collaborative UI) from the fast MVP runtime. The removal spans coordinator Socket.IO handlers and API endpoints, viewer components and client methods, demo UI elements, type definitions, and includes comprehensive design, proposal, task, and specification-delta documentation to formalize the decision and provide implementation/verification guidance. Network binding is restricted to localhost-only for the viewer service. ChangesRemove Conflict Review from Fast MVP
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ 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 |
1 of 4 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR trims the fast-MVP baseline by removing conflict-review collaboration features (issues/highlight/annotation flows) from both the coordinator and the web viewer, restricts the viewer’s Docker port binding to loopback-only, and updates docs/OpenSpec artifacts to record the retirement in preparation for the successor fast-ifc-link-demo-loop.
Changes:
- Coordinator: remove review-bootstrap + issue/annotation-related APIs and Socket.IO handlers; adjust namespace registration signature; update dev-console UI and tests.
- Viewer: remove Issue/EventLog panels and issue types; remove review-bootstrap client paths; simplify bootstrap to rely on stream-config only; update contract verification script.
- Docs/OpenSpec: add predecessor change proposal/design/tasks and spec delta; mark collaboration flows as retired; compose viewer binds to
127.0.0.1.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web-viewer-sample/src/Window.tsx | Removes issue/review-bootstrap state and handlers; loads artifacts solely from stream-config. |
| web-viewer-sample/src/types/review.ts | Removes ReviewBootstrap type and issue dependency. |
| web-viewer-sample/src/types/issues.ts | Deletes ReviewIssue type. |
| web-viewer-sample/src/components/IssuePanel.tsx | Deletes issue list panel UI. |
| web-viewer-sample/src/components/EventLogPanel.tsx | Deletes live event log panel UI. |
| web-viewer-sample/src/components/DemoControlPanel.tsx | Makes retired demo callbacks optional (with defaults). |
| web-viewer-sample/src/clients/reviewSocket.ts | Removes highlight/selection/annotation emit helpers from client interface. |
| web-viewer-sample/src/clients/demoDefaults.ts | Removes demo issue/highlight constants and helper builder. |
| web-viewer-sample/src/clients/coordinatorClient.ts | Removes getReviewBootstrap API client method. |
| web-viewer-sample/src/clients/bimControlClient.ts | Removes getReviewIssues API client method. |
| web-viewer-sample/scripts/verify-session-first-contract.mjs | Updates contract assertions to enforce session-first → stream-config and absence of review-bootstrap/issue paths. |
| compose.host-kit.yml | Binds viewer port to 127.0.0.1:${VIEWER_PORT:-5173}:5173. |
| bim-review-coordinator/src/app.ts | Removes review-bootstrap endpoint + safeIssues helper; updates registerReviewNamespace call signature. |
| bim-review-coordinator/src/socket/reviewNamespace.ts | Removes highlight/selection/annotation handlers and downstream bim-control dependency. |
| bim-review-coordinator/src/services/bimControlClient.ts | Removes issue fetch + annotation persistence methods. |
| bim-review-coordinator/src/public/dev-console.html | Removes step ④/⑤ and issue/annotation guided cards + raw emit buttons. |
| bim-review-coordinator/src/public/dev-console.js | Removes emitHighlight/emitSelection/emitAnnotation functions. |
| bim-review-coordinator/src/types.ts | Removes coordinator-side ReviewIssue type. |
| bim-review-coordinator/tests/sessions.test.ts | Updates socket validation tests to focus on joinSession. |
| bim-review-coordinator/tests/dev-console.test.ts | Updates UI assertions to ensure retired collaboration UI is absent and step count is 3. |
| bim-review-coordinator/CLAUDE.md | Adds retirement note for removed collaboration flows/endpoints. |
| AGENTS.md | Marks collaboration and issue visualization sections as retired for fast MVP. |
| openspec/changes/remove-conflict-review-from-fast-mvp/proposal.md | Documents rationale, scope, and impact for the predecessor change. |
| openspec/changes/remove-conflict-review-from-fast-mvp/design.md | Captures design decisions, verification levels, and risk notes. |
| openspec/changes/remove-conflict-review-from-fast-mvp/tasks.md | Adds task checklist for implementation/verification/archival workflow. |
| openspec/changes/remove-conflict-review-from-fast-mvp/specs/review-session-request-lifecycle/spec.md | Spec delta: records MODIFIED requirement with implementation-status note about retirement. |
| docs/superpowers/specs/2026-05-21-fast-mvp-loop-overall-design.md | Adds brainstorming overall design doc tying predecessor/successor plan together. |
Comments suppressed due to low confidence (1)
web-viewer-sample/src/components/DemoControlPanel.tsx:232
- Similarly, defaulting the optional demo actions (
onHighlightWorld,onEmitCoordinatorHighlight,onCreateAnnotation) to no-ops leaves their UI actions callable but ineffective. It’s safer to gate the rendering/disabled state of the related buttons/steps on the handler being provided so the UI reflects what’s actually supported in this build.
onHighlightWorld = () => undefined,
onFocusWorld,
onClearHighlight,
onEmitCoordinatorHighlight = () => undefined,
onCreateAnnotation = () => undefined,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
13
to
+20
| export function registerReviewNamespace( | ||
| io: Server, | ||
| store: SessionStore, | ||
| eventLog: EventLog, | ||
| bimControlClient: BimControlClient, | ||
| ): void { | ||
| // eventLog 保留為 future lifecycle audit 拓展,join/leave/heartbeat 路徑暫未直接寫入。 | ||
| void eventLog; | ||
|
|
Comment on lines
+223
to
227
| onLoadBootstrap = () => undefined, | ||
| onConnectSocket, | ||
| onOpenStage, | ||
| onLoadingState, | ||
| onGetChildren, |
|
|
||
| > **文件性質**:brainstorming `superpowers/brainstorming` skill 階段的 design artifact,不是 OpenSpec spec(authoritative spec 在 `openspec/changes/<change-id>/`)。 | ||
| > **日期**:2026-05-21 | ||
| > **發起人**:使用者(`xshiujj@gmail.com`) |
monkey1sai
added a commit
that referenced
this pull request
May 21, 2026
…s + roadmap (#91) PR #90 implementation merged (squash 9e57015) 之後的 sync/archive cleanup。 - git mv openspec/changes/remove-conflict-review-from-fast-mvp/ → openspec/changes/archive/2026-05-21-remove-conflict-review-from-fast-mvp/ - openspec/specs/review-session-request-lifecycle/spec.md 加 implementation status note 至 "Coordinator exposes lifecycle event audit log" 主 requirement(對齊 change 內 ## MODIFIED Requirements;exclusion wording 保留作 archive compatibility) - docs/plans/AI-BIM-governance-saas-roadmap-2026-05.md 加 archive 摘要(AGENTS.md §1.6;26 capability 保持 26,1 modified,fast-mvp loop predecessor gate 已開) openspec validate --specs --strict = 26 passed / 0 failed NoSuccessorWhilePredecessorOpen gate cleared → 進 successor `fast-ifc-link-demo-loop`。 Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
![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: eee08bd2e9
ℹ️ 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".
Comment on lines
+228
to
+232
| onHighlightWorld = () => undefined, | ||
| onFocusWorld, | ||
| onClearHighlight, | ||
| onEmitCoordinatorHighlight, | ||
| onCreateAnnotation, | ||
| onEmitCoordinatorHighlight = () => undefined, | ||
| onCreateAnnotation = () => undefined, |
There was a problem hiding this comment.
Defaulting onHighlightWorld, onEmitCoordinatorHighlight, and onCreateAnnotation to no-op functions means the panel still presents actionable review controls even when the caller does not wire them. In this commit, Window.tsx stops passing those handlers, so these buttons silently do nothing while UI text still implies real Socket.IO/DataChannel side effects, which can mislead manual demo/verification runs into thinking review actions were exercised when no request/event was sent.
Useful? React with 👍 / 👎.
This was referenced May 21, 2026
monkey1sai
added a commit
that referenced
this pull request
May 21, 2026
) PR #92 (implementation, squash fb9dea3) merged 之後的 OpenSpec sync/archive cleanup。 - git mv openspec/changes/fast-ifc-link-demo-loop/ → openspec/changes/archive/2026-05-21-fast-ifc-link-demo-loop/ - openspec/specs/{local-coordinator-ifc-ready-intake-boundary, conversion-webhook-lifecycle, demo-fast-mvp-orchestration, documentation-source-of-truth}/spec.md 加 implementation status note 引用 archive folder 內完整 ADD requirement bodies - docs/plans/AI-BIM-governance-saas-roadmap-2026-05.md 加 archive 摘要(AGENTS.md §1.6;26 capability 保持 26,4 modified status notes,fast-mvp loop predecessor + successor 完整) openspec validate --specs --strict = 26 passed / 0 failed fast-mvp loop 兩段 archive 完成: - predecessor `remove-conflict-review-from-fast-mvp`:PR #90/#91 (9e57015 / 4c892d0) - successor `fast-ifc-link-demo-loop`:PR #92 (fb9dea3) Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced Jun 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Predecessor change
remove-conflict-review-from-fast-mvpfor the fast-ifc-link-demo-loop。純減法 + 一行 compose port bind + docs/spec 退役 note。為 successorfast-ifc-link-demo-loop(同機 IFC ifc-ready → 同步下載 → 轉檔 → viewer 連結點開看 stream)清乾淨 baseline。詳細設計見
docs/superpowers/specs/2026-05-21-fast-mvp-loop-overall-design.md與openspec/changes/remove-conflict-review-from-fast-mvp/{proposal,design,tasks}.md。What changed
getReviewIssues/createAnnotation/review-bootstrapendpoint、registerReviewNamespace改 signature、/uistep bar 5→3 + guided cards 衝突檢討 3 張刪 + raw Socket emit 按鈕刪、dev-console.test.ts / sessions.test.ts 對齊IssuePanel/EventLogPanel/types/issues整檔、reviewSocket emit{Highlight,Selection,Annotation} 拆掉、coordinatorClient.getReviewBootstrap刪、Window.tsx 清 issue state/methods/render、DemoControlPanel 4 個 issue/annotation props 改 optional + default、verify-session-first-contract 改驗 session-first → stream-configcompose.host-kit.ymlviewer.ports改127.0.0.1:5173:5173(對應 memorywebrtc-1on1-entrypoint-via-coordinator-ui的 1:1 邊界)AGENTS.md§5.4 / §5.5 / §7.4 +bim-review-coordinator/CLAUDE.mdRole 段加退役 notereview-session-request-lifecycle## MODIFIED Coordinator exposes lifecycle event audit log,加 implementation status note(collaboration event 排除清單 wording 保留作 archive compatibility)23 files changed / +96 / -625 lines
Blast radius
GitNexus pre-impact:
registerReviewNamespace/IssuePanel/DemoControlPanel全 LOW(impactedCount=0)。無 HIGH/CRITICAL。Verification(5 級)
npm run verifynpm run buildnpm run test:session-firstpytest testsopenspec validate remove-conflict-review-from-fast-mvp --strictopenspec validate --specs --strictgit diff --stat影響面docker compose up --build coordinator viewernetstat5173127.0.0.1:5173docker exec coordinator /healthdocker exec /ui字串斷言mcp__claude-in-chromenavigate/uiTest plan
openspec/changes/archive/2026-05-21-remove-conflict-review-from-fast-mvp/docs/plans/AI-BIM-governance-saas-roadmap-2026-05.md(AGENTS.md §1.6)fast-ifc-link-demo-loop🤖 Generated with Claude Code
Summary by CodeRabbit
Changes
Documentation