Repository navigation
修正 A1 選檔後可直接執行治理檢核 - #291
Conversation
📝 WalkthroughWalkthroughThis PR decouples governance rule-run execution from review session selection in the A1 workflow ("A1 v2"). The state machine removes the PICK_SESSION event and gates RUN on step progress plus a valid ifcPath. The workbench page calls createRuleRun directly against the selected IFC, with review sessions now optional for 3D Review Room handoff. UI copy and extensive test suites are updated accordingly. ChangesA1 v2 run decoupling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant A1GovernanceWorkbenchPage
participant a1Reducer
participant governanceClient
User->>A1GovernanceWorkbenchPage: pick IFC model
A1GovernanceWorkbenchPage->>a1Reducer: dispatch PICK_FILE (ifcPath)
User->>A1GovernanceWorkbenchPage: click run validation
A1GovernanceWorkbenchPage->>a1Reducer: dispatch RUN
a1Reducer-->>A1GovernanceWorkbenchPage: allow (step != idle, ifcPath truthy)
A1GovernanceWorkbenchPage->>governanceClient: createRuleRun(ifc_source_path, ids_path)
governanceClient-->>A1GovernanceWorkbenchPage: run result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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: ba726537bc
ℹ️ 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".
| const { rule_run_id } = await governanceClient.createRuleRun({ | ||
| ifc_source_path: state.ifcPath, | ||
| ids_path: idsPath || undefined, |
There was a problem hiding this comment.
When the user follows the updated A1 flow, state.ifcPath is the selected MinIO object key from /api/minio/objects (for example 松風庵/root/main/u1/model.ifc), but POST /api/governance/rule-runs is a direct pass-through that requires a server-side filesystem path and governance-service rejects anything for which os.path.exists(ifc_source_path) is false. This makes the new “select IFC then run” path fail with ifc_source_path not found; the previous for-session route resolved the session to a host-local IFC path before forwarding.
Useful? React with 👍 / 👎.
| @@ -637,14 +632,13 @@ export function A1GovernanceWorkbenchPage() { | |||
| const nextSession = e.target.value; | |||
| if (nextSession === selectedSession) return; | |||
| setSelectedSession(nextSession); | |||
There was a problem hiding this comment.
If an operator runs validation while session A is selected, the success path can enrich state.failed with USD prim paths from A's expected_mapping_url; this handler then lets them switch to session B without clearing those results, so the Review Room handoff is built for B while still carrying A's usd_prim_path. In environments with multiple active sessions for different converted stages, that can highlight the wrong prim/stage; clear or re-enrich the stored results on session changes.
Useful? React with 👍 / 👎.
| export type A1Event = | ||
| | { type: "PICK_FILE"; ifcPath: string } | ||
| | { type: "PICK_SESSION" } | ||
| | { type: "RUN" } |
There was a problem hiding this comment.
After this event union drops PICK_SESSION, src/console/a1Machine.test.ts still dispatches { type: "PICK_SESSION" } in two tests that expect the reducer to enter picked/running; running npm test -- a1Machine.test.ts now fails both tests, so the package-level npm run verify gate will fail even though the narrower tests listed in the commit message pass. Please update or remove those stale reducer tests as part of this behavior change.
Useful? React with 👍 / 👎.
| <Btn primary data-testid="a1-step-run" disabled={state.step === "idle" || !state.ifcPath || (state.step === "running" && !state.runError)} | ||
| caption={state.ifcPath ? "POST /api/governance/rule-runs" : t("先選定 IFC 模型；不需要 review session 即可檢核", "Select an IFC model first; review session is not required for validation")} onClick={doRun}> |
There was a problem hiding this comment.
After locking model A, changing the MinIO dropdown to model B leaves state.ifcPath set to A while the run button remains enabled because it only checks that state.ifcPath is non-empty; clicking “Run Rule Validation” then submits the stale locked model rather than the model currently shown in the selector. This mismatch matters now that the run path uses state.ifcPath directly, so the dropdown change should clear the locked file or the button should require selectedKey === state.ifcPath.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR reworks the A1 governance workbench (web-viewer-sample) so operators can run rule validation immediately after selecting an IFC, rather than being gated on first creating/selecting a review session. The A1Event state machine drops the PICK_SESSION event, RUN gating switches from !selectedSession to !state.ifcPath, and doRun calls governanceClient.createRuleRun({ ifc_source_path, ids_path }) instead of createRuleRunForSession. Review sessions are demoted to an optional target for 3D Review Room handoff / mapping enrichment. UI captions, the first lifecycle step label ("上傳模型" → "選檔"), and the associated tests are updated accordingly.
Changes:
- Decouple A1 rule-run from review sessions: gate on selected IFC and call the direct
POST /api/governance/rule-runsendpoint. - Remove the
PICK_SESSIONevent/reducer case and the auto-PICK_SESSIONeffect; stop resetting state when the session dropdown changes. - Update copy/labels and the
console.test.tsx/A1ViewerEmbed.test.tsxsuites (newpickModelhelper,createRuleRunmocks).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| web-viewer-sample/src/console/a1Machine.ts | Removes PICK_SESSION from the event union and reducer; RUN now guards on !state.ifcPath; comment updated for the new first step. |
| web-viewer-sample/src/console/pages.tsx | doRun calls createRuleRun with ifc_source_path: state.ifcPath; run button/gating keyed off state.ifcPath; captions and session panel copy updated; session-change no longer dispatches RESET. |
| web-viewer-sample/src/console/console.test.tsx | Swaps createRuleRunForSession mocks for createRuleRun, removes selectReviewSession helper, updates SSR assertions. |
| web-viewer-sample/src/console/A1ViewerEmbed.test.tsx | Adds pickModel helper; asserts run enables via picked IFC and calls createRuleRun without a session. |
Two blocking concerns were found: a1Machine.test.ts (not part of this PR) still references the removed PICK_SESSION event, breaking type-checking and the reducer tests; and the direct createRuleRun call forwards a MinIO object key as ifc_source_path, which governance-service rejects with HTTP 400 since it expects a resolvable local filesystem path. Both are detailed in inline comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case "RUN": | ||
| // 守門改看 step(非 idle 才前進):picked 可能來自 PICK_FILE(ifcPath 有值)或 PICK_SESSION | ||
| // (ifcPath 為空),故不可再用 !state.ifcPath 當「未選檔」判斷,否則 session-pick 後 RUN 會被誤擋。 | ||
| if (state.step === "idle") return state; | ||
| if (state.step === "idle" || !state.ifcPath) return state; |
| const { rule_run_id } = await governanceClient.createRuleRun({ | ||
| ifc_source_path: state.ifcPath, | ||
| ids_path: idsPath || undefined, | ||
| }); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c46989786
ℹ️ 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".
| try { | ||
| const { rule_run_id } = await governanceClient.createRuleRunForSession(selectedSession, { ids_path: idsPath || undefined }); | ||
| const { rule_run_id } = await governanceClient.createRuleRun({ | ||
| ifc_source_path: state.ifcPath, |
There was a problem hiding this comment.
When a review session is selected but belongs to a different converted stage than the locked MinIO key, this new direct run targets state.ifcPath while the later success path still enriches results from selectedSession's expected_mapping_url; because the session dropdown is global and is not filtered or validated against the locked IFC, Review Room can receive USD prim paths from the wrong stage. Clear or validate selectedSession when the IFC is locked, or only enrich/handoff when the session is known to correspond to the same model.
Useful? React with 👍 / 👎.
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff79254d68
ℹ️ 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".
| const { rule_run_id } = await governanceClient.createRuleRun({ | ||
| ifc_source_path: state.ifcPath, | ||
| ids_path: idsPath || undefined, | ||
| }); |
There was a problem hiding this comment.
When the operator follows the conversion-ready flow, A1 auto-selects the review session for the converted model, but this new direct rule-run body only sends ifc_source_path/ids_path. The previous for-session proxy populated model_version_id from that session before forwarding, and issues_from_rule_run later copies run.model_version_id into every formal issue; with this call those issues (and downstream BCF/diff-impact filtering by model version) are created unbound even though the matching session is available. Include the selected session's model_version_id when it corresponds to the locked model, or keep a resolving endpoint that preserves that field.
Useful? React with 👍 / 👎.
| <Btn primary data-testid="a1-step-run" disabled={state.step === "idle" || !state.ifcPath || (state.step === "running" && !state.runError)} | ||
| caption={state.ifcPath ? "POST /api/governance/rule-runs" : t("先選定 IFC 模型；不需要 review session 即可檢核", "Select an IFC model first; review session is not required for validation")} onClick={doRun}> |
There was a problem hiding this comment.
In the no-active-session path, a1-trigger-convert is enabled by selectedKey, so an operator can select a MinIO object and queue conversion without clicking the separate “Select Model” button. After conversion becomes ready, the session is auto-selected, but the removed PICK_SESSION flow no longer moves the reducer out of idle, and this run button still requires state.ifcPath; the validation step stays disabled until the user goes back and locks the same file. Either lock the selected key when starting/finishing conversion, or require the conversion button to use an already locked IFC.
Useful? React with 👍 / 👎.
摘要
POST /api/governance/rule-runs。createRuleRunForSessiongating。a1Machine.test.ts，移除舊PICK_SESSIONfor-session 契約，改驗證 A1 v2 必須以已選 IFC 作為 CPU rule-run 目標。docs/superpowers/specs/2026-07-03-a1-direct-ifc-rule-run.md。驗證
npm test -- A1ViewerEmbed.test.tsx console.test.tsxnpm test -- a1Machine.test.tsnpm run buildnpm run verifygit diff --cached --checkimpact(a1Reducer, upstream, includeTests=true)：riskLOWdetect_changes(scope=staged)：第一次 implementation riskmedium，第二次 test-only risklow，spec evidence docs-only risklow，無 HIGH/CRITICALFrontend Verification
http://127.0.0.1:8004/ui/#a1（本 PR 未重啟測試部署區實機驗證）a1-step-pick,a1-step-runvia Vitest/jsdom松風庵/root/main/u1/model.ifc+ sample IDS pathnpm run verify覆蓋 viewer build、Vitest、struct-logcreateRuleRun呼叫與createRuleRunForSession不被呼叫