Repository navigation
fix(console): 修正 A1 rule-run 來源選取與 BCF 面板 - #302
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7ESqHTEqCYE8PnzRHmRZc
交叉對抗審批：5 視角 finder × 每 major finding 2-lens 反駁（Sonnet 5、39 agents）。 主要修正：IN download_status/conversion_authority 兩欄補列佇列表；Pipeline Panel （含 concurrency NOT BUILT 誠實揭露）對映至摘要卡展開細節；七軸 spec N1 正式覆寫 聲明＋使用者裁決紀錄；摘要統計口徑標示；三源串接主鍵改 idempotency_key；共用件 export 前置步驟；alias 重導限 useEffect；MD 對外 handoff source 統一 minio。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7ESqHTEqCYE8PnzRHmRZc
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7ESqHTEqCYE8PnzRHmRZc
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W7ESqHTEqCYE8PnzRHmRZc
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds planning/spec documents for merging the conversion, intake, and MinIO console pages into a unified Model Data (MD) workbench, and refactors the A1 governance workbench to support local_fs vs MinIO source selection, backed by a new shared conversion UI module and extended governanceClient APIs with corresponding test updates. ChangesModel Data Merge Planning Documents
Estimated code review effort: 3 (Moderate) | ~25 minutes A1 Source Picker and Shared Conversion UI Implementation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant A1GovernanceWorkbenchPage
participant governanceClient
participant a1Reducer
User->>A1GovernanceWorkbenchPage: select source_kind (local_fs or MinIO)
A1GovernanceWorkbenchPage->>governanceClient: filesTree()
governanceClient-->>A1GovernanceWorkbenchPage: FilesTreeResponse
A1GovernanceWorkbenchPage->>a1Reducer: PICK_FILE(ifcPath, modelVersionId)
a1Reducer-->>A1GovernanceWorkbenchPage: updated A1State (modelVersionId set)
User->>A1GovernanceWorkbenchPage: trigger rule run
A1GovernanceWorkbenchPage->>governanceClient: createRuleRun(ifc_source_path, model_version_id)
governanceClient-->>A1GovernanceWorkbenchPage: rule run result
A1GovernanceWorkbenchPage->>governanceClient: listIssues(status, filters)
governanceClient-->>A1GovernanceWorkbenchPage: IssueRow[]
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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: 6ce438a1c8
ℹ️ 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".
| <select data-testid="a1-localfs-select" className="ec-btn" style={{ minWidth: 520 }} | ||
| disabled={fsTree === null || Boolean(fsErr)} | ||
| value={selectedLocalPath} | ||
| onChange={(e) => setSelectedLocalPath(e.target.value)}> |
There was a problem hiding this comment.
When an operator has already clicked “Select Model” for local file A, this onChange only updates selectedLocalPath; it leaves the reducer's locked state.ifcPath as A and the Run button stays enabled. If they then choose local file B in the dropdown and click Run without pressing “Select Model” again, POST /api/governance/rule-runs still validates A while the visible picker shows B, so the rule-run can be created for the wrong model. Reset the picked state on dropdown changes or disable Run until the new selection is explicitly locked.
Useful? React with 👍 / 👎.
| } else { | ||
| setA1Issues((current) => current); | ||
| } |
There was a problem hiding this comment.
In the idempotent path, issues/from-rule-run returns issue_ids: [] for already-created issues, so this branch leaves the current topic list unchanged. If the first create succeeded but the per-issue fetch failed, or the issues were created from another client, the current list is empty; clicking “Create Issues” again still dispatches success with zero created issues and the BCF panel never shows the existing topics. Treat skipped/empty issue_ids as a signal to reload existing issues for the run, or surface the fetch failure instead of silently preserving an empty list.
Useful? React with 👍 / 👎.
| if (!selectedLocalOption) return; | ||
| setActionErr(null); | ||
| setA1Issues([]); | ||
| dispatch({ type: "PICK_FILE", ifcPath: selectedLocalOption.version.path }); |
There was a problem hiding this comment.
This selection now knows the project/model/version (selectedLocalOption.modelVersionId is computed above), but the reducer only records the file path, so doRun later posts no model_version_id. For local_fs runs, issues/from-rule-run copies the stored rule-run model_version_id into generated issues; leaving it null makes the A1 issues/BCF output unbound to the selected model version even though the operator picked one from the versioned file library. Store and pass the selected model version along with the path.
Useful? React with 👍 / 👎.
| setActionErr(null); | ||
| setA1Issues([]); | ||
| } | ||
| setSourceKind("local_fs"); |
There was a problem hiding this comment.
When switching from a MinIO selection to local_fs, this resets the A1 state but leaves selectedKey intact. After the operator picks and runs a local_fs model, the MinIO source cross-link and #conv handoff still use that stale MinIO key, so the page can send the operator to an unrelated source object for a local filesystem validation. Clear selectedKey on this source switch or gate those links by sourceKind === "minio".
Useful? React with 👍 / 👎.
| const rows = await Promise.all(issue_ids.map((id) => governanceClient.getIssue(id))); | ||
| setA1Issues(rows); |
There was a problem hiding this comment.
The per-issue fetch runs without a generation/run guard, so a slow getIssue batch from run A can resolve after the operator resets the page or completes run B. In that case this late setA1Issues(rows) can repopulate the panel with A's topics, and the subsequent CREATE_ISSUES_OK can even mark the currently scored run as issued with A's count. Capture the run id/generation before starting the fetch and ignore results once the selected run changes or the page resets.
Useful? React with 👍 / 👎.
| <Panel title={t("交付", "Deliverables")} sub={t("開 Issue / 匯出 Excel / 匯出 BCF 2.1 走真實後端；BCF 需先建 Issue（step=issued/delivered）才 enable；3D 交給 Review Room 手動 attach / highlight", "Open Issue / Export Excel / Export BCF 2.1 go through the real backend; BCF is enabled only after Issues are created (step=issued/delivered); 3D is handed off to Review Room for manual attach / highlight")} prov="asbuilt"> | ||
| <div data-testid="a1-bcf-review-panel" style={{ marginBottom: 10 }}> | ||
| <div className="ec-grid" style={{ marginBottom: 8 }}> | ||
| <Field k="BCF topics" v={a1Issues.length > 0 ? String(a1Issues.length) : t("尚未建立本輪 Issue", "no issues created for this run yet")} prov={a1Issues.length > 0 ? "asbuilt" : "p1"} /> |
There was a problem hiding this comment.
issues/from-rule-run can return kind: "annotation" rows when a failed result has no ifc_guid, but the BCF exporter only includes formal kind=issue rows with an IFC GUID. Counting every a1Issues item as “BCF topics” makes runs with annotations show exportable topics even though the BCF download will omit them or 404 if no formal issues exist. Filter this count/list to kind === "issue" && ifc_guid, or label annotations separately from BCF topics.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR fixes the A1 governance workbench so it no longer sends a raw MinIO object key as ifc_source_path to POST /api/governance/rule-runs (which the governance service rejects with 400 because it validates os.path.exists). It introduces an explicit local_fs/MinIO source picker: local_fs (backed by GET /api/governance/files/tree) is the executable CPU rule-run source, while MinIO remains available only for source verification/handoff. It also removes A1's direct conversion triggering, adds an A1 "bridge rail" evidence panel and a BCF review-topics panel (per-issue fetch + idempotent-retry preservation), extracts shared conversion UI into a new conversionShared.tsx module, and enriches governance proxy error messages.
Changes:
- Replace the single MinIO dropdown with a
local_fs/MinIO source picker;local_fsserver-local paths drive CPU rule-run, MinIO keys are handoff-only and cannot be picked. - Remove A1 conversion queuing/polling; add A1 bridge rail + BCF review panel with
getIssue-backed topics and issue transitions. - Extract
LifecycleStrip,CoverageDrawer, ledger/role helpers intomodelData/conversionShared.tsx, and surface upstream errordetailingovernanceClient.jsonFetch.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web-viewer-sample/src/console/pages.tsx | Core A1 refactor: source picker, removal of conversion triggering, bridge rail + BCF panel, shared-component import. |
| web-viewer-sample/src/console/modelData/conversionShared.tsx | New module holding the conversion UI helpers moved out of pages.tsx (single source of truth). |
| web-viewer-sample/src/console/governanceClient.ts | Adds getIssue; extracts backend error detail/error/reason into thrown message. |
| web-viewer-sample/src/console/A1ViewerEmbed.test.tsx | Updates fixtures/helpers for local_fs selection; adds MinIO-not-sent, source-switch, BCF-retry, and no-conversion tests. |
| web-viewer-sample/src/console/console.test.tsx | Adds filesTree mock + fixture; asserts new source-picker/bridge/BCF testids and local_fs pick flow. |
| web-viewer-sample/src/console/incomingHandoff.test.tsx | Mocks filesTree; asserts verified MinIO handoff seeds the selector but keeps direct CPU pick disabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| seededHandoffKeyRef.current = key; | ||
| setSourceKind("minio"); | ||
| setSelectedKey(key); |
| projectId: project.project_id, | ||
| modelId: model.model_id, | ||
| version, | ||
| modelVersionId: `${project.project_id}/${model.model_id}/${version.name}`, |
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/superpowers/specs/2026-07-06-model-data-conversion-merge-design.md (1)
348-355: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win把選取樣式縮到 MD 區塊內。
[data-selected="true"]是全域 selector，會把任何共用這個 attribute 的元件一起套上 MD 的外框樣式；這很容易和既有 console 元件互相污染。建議修正
-[data-selected="true"] { outline: 1px solid var(--ec-accent, `#7fd962`); border-radius: 4px; } +.md-split-tree [data-selected="true"] { outline: 1px solid var(--ec-accent, `#7fd962`); border-radius: 4px; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-07-06-model-data-conversion-merge-design.md` around lines 348 - 355, Restrict the selected-state styling so it only applies inside the MD block, since the current [data-selected="true"] selector is global and can leak styles into other shared components. Update the MD-related selector(s) in the design spec to scope the rule under the MD container/class used in this section, and ensure any style references tied to the MD border/outline behavior remain localized to that component.
🧹 Nitpick comments (1)
web-viewer-sample/src/console/A1ViewerEmbed.test.tsx (1)
69-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded, machine-specific absolute path duplicated across test files.
LOCAL_IFC_PATH/LOCAL_IFC_PATH_Bbake in a developer-specific Windows path (C:/Repos/active/iot/AI-BIM-governance/storage/...). The same literal path is duplicated inconsole.test.tsx(A1_LOCAL_IFC_PATH). Since these values are only used as opaque mock data (never touch the real filesystem), a portable placeholder (e.g./storage/270/建築/model.ifc) shared via a common test fixtures module would be clearer and avoid drift between the two copies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-viewer-sample/src/console/A1ViewerEmbed.test.tsx` around lines 69 - 100, The test fixture paths in A1ViewerEmbed.test.tsx are hardcoded machine-specific Windows absolutes and duplicated elsewhere, so replace LOCAL_IFC_PATH and LOCAL_IFC_PATH_B with portable mock path placeholders and centralize them in a shared test fixture module. Update the fakeFilesTree setup in A1ViewerEmbed.test.tsx and the matching A1_LOCAL_IFC_PATH usage in console.test.tsx to import the same shared constants so the mock data stays consistent and doesn’t drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-07-06-model-data-conversion-merge.md`:
- Around line 274-275: The plan should explicitly include
useConversionActions.ts as a required implementation artifact, since Task 5
depends on the shared action hook for both panes. Update the File Structure /
Files sections to name useConversionActions.ts alongside the existing action and
dialog work, and make sure the plan states that Task 4 and Task 5 will both
reuse this hook rather than duplicating runAction or dialog logic.
- Around line 277-278: The Step 4 and Step 5 checklist items are misindented and
are being parsed as a new top-level list instead of staying nested under Task 5.
Update the markdown list structure in the plan so the Step 4/5 items remain at
the same nested level as the other Task 5 steps, keeping the checklist hierarchy
consistent and markdownlint-compliant.
In `@docs/superpowers/specs/2026-07-06-model-data-conversion-merge-design.md`:
- Around line 126-130: Legacy alias redirects for `#conv` and `#intake` currently
only rewrite the hash to `#minio`, but the resulting URL still lacks a valid
handoff payload for parseHandoff(). Update the alias handling in the MD page
flow so the redirect or follow-up parsing preserves a legal handoff by injecting
the required source=minio (or by explicitly recognizing the legacy bare-query
form) before validation/highlight runs, keeping the behavior aligned with the
existing useEffect-based redirect and the minio page’s handoff logic.
---
Outside diff comments:
In `@docs/superpowers/specs/2026-07-06-model-data-conversion-merge-design.md`:
- Around line 348-355: Restrict the selected-state styling so it only applies
inside the MD block, since the current [data-selected="true"] selector is global
and can leak styles into other shared components. Update the MD-related
selector(s) in the design spec to scope the rule under the MD container/class
used in this section, and ensure any style references tied to the MD
border/outline behavior remain localized to that component.
---
Nitpick comments:
In `@web-viewer-sample/src/console/A1ViewerEmbed.test.tsx`:
- Around line 69-100: The test fixture paths in A1ViewerEmbed.test.tsx are
hardcoded machine-specific Windows absolutes and duplicated elsewhere, so
replace LOCAL_IFC_PATH and LOCAL_IFC_PATH_B with portable mock path placeholders
and centralize them in a shared test fixture module. Update the fakeFilesTree
setup in A1ViewerEmbed.test.tsx and the matching A1_LOCAL_IFC_PATH usage in
console.test.tsx to import the same shared constants so the mock data stays
consistent and doesn’t drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 89f988ea-2115-4f46-b344-bbe60f92be18
⛔ Files ignored due to path filters (1)
docs/plans/nvidia-cosmos-diagram.jpgis excluded by!**/*.jpg
📒 Files selected for processing (11)
AGENTS.mdCLAUDE.mddocs/superpowers/plans/2026-07-06-model-data-conversion-merge.mddocs/superpowers/specs/2026-07-06-model-data-conversion-merge-design.mdweb-viewer-sample/src/console/A1ViewerEmbed.test.tsxweb-viewer-sample/src/console/a1Machine.tsweb-viewer-sample/src/console/console.test.tsxweb-viewer-sample/src/console/governanceClient.tsweb-viewer-sample/src/console/incomingHandoff.test.tsxweb-viewer-sample/src/console/modelData/conversionShared.tsxweb-viewer-sample/src/console/pages.tsx
| - 動作區：「觸發轉檔」鈕（`disabled={!["untracked","failed","indeterminate"].includes(chip)}`、intent→confirm `IntentDialog`＋`confirmTrigger` 邏輯搬 M 頁 1884-1901 原文、成功後 `void data.loadRecords()`）；job 存在時依 `job.status` 掛插隊/重試鈕（gating 條件原文搬 CV 1365-1383，動作走與 Task 4 相同的 `conversionPrioritize`/`conversionRetry`＋dialog——為避免雙份 dialog 邏輯，把 Task 4 的 `runAction`＋dialog 抽成本目錄共用 hook `useConversionActions.ts`，兩 pane 共用；Task 4 實作時即建此檔）。 | ||
| - coverage 區：`job?.conversion_job_id` 存在→「coverage」展開鈕＋`CoverageDrawer`（快取/載入鎖邏輯搬 CV `toggleCoverage` 1025-1051 原文，state 本地持有）＋IN 品質誠實文案精簡三行（quality_metrics 為 pass-through artifact／不承諾精準 GUID／無遙測欄位標未取得——文字取自 pages.tsx:3276-3281 的 Field 值）。 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
把 useConversionActions.ts 明確列進計畫。
Task 5 已經依賴這個共用 hook，但 File Structure / Files: 區塊都沒有把它列成實作檔案，會讓後續落地時漏掉一個必要產物。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-07-06-model-data-conversion-merge.md` around
lines 274 - 275, The plan should explicitly include useConversionActions.ts as a
required implementation artifact, since Task 5 depends on the shared action hook
for both panes. Update the File Structure / Files sections to name
useConversionActions.ts alongside the existing action and dialog work, and make
sure the plan states that Task 4 and Task 5 will both reuse this hook rather
than duplicating runAction or dialog logic.
| - [ ] **Step 4: 跑測試確認 PASS**。 | ||
| - [ ] **Step 5: Commit**——`git commit -m "feat(console): ObjectDetailPane 單檔詳情（MD 合一 Task 5）"` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
修正這兩個清單縮排。
這裡的 Step 4/5 目前脫離了 Task 5 的巢狀層級，markdownlint 已經標出問題；照現在的寫法會變成另一組頂層清單。
建議修正
- - [ ] **Step 4: 跑測試確認 PASS**。
- - [ ] **Step 5: Commit**——`git commit -m "feat(console): ObjectDetailPane 單檔詳情（MD 合一 Task 5）"`
+ - [ ] **Step 4: 跑測試確認 PASS**。
+ - [ ] **Step 5: Commit**——`git commit -m "feat(console): ObjectDetailPane 單檔詳情（MD 合一 Task 5）"`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [ ] **Step 4: 跑測試確認 PASS**。 | |
| - [ ] **Step 5: Commit**——`git commit -m "feat(console): ObjectDetailPane 單檔詳情（MD 合一 Task 5）"` | |
| - [ ] **Step 4: 跑測試確認 PASS**。 | |
| - [ ] **Step 5: Commit**——`git commit -m "feat(console): ObjectDetailPane 單檔詳情（MD 合一 Task 5）"` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 277-277: Inconsistent indentation for list items at the same level
Expected: 2; Actual: 0
(MD005, list-indent)
[warning] 278-278: Inconsistent indentation for list items at the same level
Expected: 2; Actual: 0
(MD005, list-indent)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-07-06-model-data-conversion-merge.md` around
lines 277 - 278, The Step 4 and Step 5 checklist items are misindented and are
being parsed as a new top-level list instead of staying nested under Task 5.
Update the markdown list structure in the plan so the Step 4/5 items remain at
the same nested level as the other Task 5 steps, keeping the checklist hierarchy
consistent and markdownlint-compliant.
Source: Linters/SAST tools
| - `#minio` ＝新頁 MD。`#conv`、`#intake` 改為**重導 alias**：以 `window.location.replace` 導向 `#minio` 並**保留 query string**（handoff id 由新頁照 §3.4 重驗）。重導**必須放在 `useEffect`**、不得在 render 期間執行（既有 `console.test.tsx` 以 `renderToString` 同步斷言純渲染，render 期副作用會污染測試慣例）。明示差異：這是本 repo 第一個「URL 重寫式」alias——既有 alias（`coordinator`／`semantic`／`overview` 等）是同 hash 直接 render 對應元件、網址列不變；`#conv`／`#intake` 改寫網址列為 `#minio` 是刻意設計（單一正典 URL），已含於使用者 2026-07-06 合併裁決（§9）。依 `docs/plans/docs-plans-README.md` deep-link aliases 保留原則，路由不砍。 | ||
| - `data.ts` `PAGES`：移除 `conv`、`intake` 兩項；`minio` 項改 `no: "MD"`、`label: "模型資料與轉檔"`。`NAV_LABEL`：`minio: { tech: "Model Data & Conversion", biz: "模型資料與轉檔" }`；`conv`／`intake` 條目保留（alias 期間 title 仍可解析）。 | ||
| - FlowBar：①接收建模來源、②自動轉換 3D 改 `page: "minio"`。 | ||
| - `COPILOT_PROMPTS`：`conv` 與 `minio` 條目合併至 `minio`。 | ||
| - `handoff.ts` `AxisKey` 七軸型別**不變**（舊 URL parse 相容）；`EdgeConsole` `AXIS_SET`→`railAxis` 對映將 `conv`／`intake` 歸到 `minio`。發送端更新：`IssuesRuleCenterPage`（A1）等頁的 `buildHandoff("conv", …)` 改 target `"minio"`（payload 欄位不變）；`buildHandoff("minio", …)` 照舊。**MD 頁自身對外送出 handoff 時 `source` 一律填 `"minio"`**（原 CV／IN 程式碼搬移時同步更新原本的 `source:"conv"`／`source:"intake"`——`source` 是接收端原樣顯示給使用者的來源標籤，不得指向已不存在的獨立頁）。 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
補上 legacy alias 的有效 handoff 正規化。
parseHandoff() 只接受帶 source 的 hash；照目前寫法，#conv?job_id=… / #intake?job_id=… 只會被改寫成 #minio?...，但仍然不是有效 handoff，後面的驗證與高亮流程會直接失效。
如果這些舊連結要真的可用，重導時就要補成合法 payload（例如明確注入 source=minio），或另外在 MD 頁辨識 legacy bare-query case。
Also applies to: 179-180
🧰 Tools
🪛 LanguageTool
[style] ~127-~127: The word ‘biz’ is informal. Consider replacing it.
Context: ...o: "MD"、label: "模型資料與轉檔"。NAV_LABEL：minio: { tech: "Model Data & Conversion", biz: "模型資料與轉檔" }；conv／intake` 條目保留（alia...
(BIZ)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-07-06-model-data-conversion-merge-design.md`
around lines 126 - 130, Legacy alias redirects for `#conv` and `#intake` currently
only rewrite the hash to `#minio`, but the resulting URL still lacks a valid
handoff payload for parseHandoff(). Update the alias handling in the MD page
flow so the redirect or follow-up parsing preserves a legal handoff by injecting
the required source=minio (or by explicitly recognizing the legacy bare-query
form) before validation/highlight runs, keeping the behavior aligned with the
existing useEffect-based redirect and the minio page’s handoff logic.
Source: Linked repositories
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 354e612cf0
ℹ️ 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 existingRows = await governanceClient.listIssues(undefined, { | ||
| model_version_id: guardedModelVersionId, | ||
| kind: "issue", | ||
| }); |
There was a problem hiding this comment.
When the current run has no failed results, or when its issues cannot be reloaded, this empty-issue_ids path queries only by model_version_id and then accepts any rule_result issue for that model. If the same model has older A1 issues from a previous run, clicking “Create Issues” on the new clean/current run repopulates the BCF panel with stale topics and can enable export for the wrong findings; correlate the reload to the current run's result/source refs, or do not fall back to all model-version issues.
Useful? React with 👍 / 👎.
| // F1：bcfEnabled 同時檢查 issuesCreated（獨立追蹤「曾真正建過 Issue」）與 step。 | ||
| // scored→EXPORT_OK→delivered 不經 CREATE_ISSUES_OK，issuesCreated 仍 false → BCF disabled。 | ||
| const bcfEnabled = state.issuesCreated && (state.step === "issued" || state.step === "delivered"); | ||
| const bcfEnabled = state.issuesCreated && bcfIssues.length > 0 && (state.step === "issued" || state.step === "delivered"); |
There was a problem hiding this comment.
When another model already has formal issues in the shared issue store, this enables A1 export based on the current panel's bcfIssues, but the click handler still calls governanceClient.bcfExportUrl() without model_version_id, so /api/bcf/export downloads every formal issue rather than the topics shown for this run/model. Pass the selected state.modelVersionId (or otherwise scope by the current run) when enabling/exporting.
Useful? React with 👍 / 👎.
| </Btn> | ||
| {convJobId && <span className="ec-s" data-testid="a1-convert-job">job: {convJobId}</span>} | ||
| <a className="ec-s" data-testid="a1-conv-link" href={buildHandoff("conv", { source: "a1", job_id: convJobId ?? undefined })}>{t("到 IFC→USD 轉檔排程查看詳情 →", "View details in the conversion schedule →")}</a> | ||
| <a className="ec-s" data-testid="a1-conv-link" href={buildHandoff("conv", { source: "a1", minio_key: sourceKind === "minio" ? selectedKey || undefined : undefined })}>{t("到 IFC→USD 轉檔排程查看詳情 →", "View details in the conversion schedule →")}</a> |
There was a problem hiding this comment.
When the selected MinIO object has not been recorded in the conversion ledger yet, this sends #conv?minio_key=..., but the #conv receiver validates minio_key only against conversion records, not the MinIO object list. The operator has just selected a valid source object, yet the target page reports it as not found and has no row from which to trigger conversion; route untracked objects to the MinIO page or provide a trigger-by-key path.
Useful? React with 👍 / 👎.
Summary
mainso the A1 rule-run fix can actually land inorigin/mainand be rebuilt into the deployment checkout.GET /api/governance/files/treeinstead of sending a raw MinIO object key asifc_source_path.Root Cause
The demo failure was caused by the A1 UI sending the MinIO object key from
selectedKeyintoPOST /api/governance/rule-runsasifc_source_path. The governance service validatesos.path.exists(req.ifc_source_path), so object keys are rejected with400 Bad Request/ifc_source_path not found.AI Coding Governance
governance proxy /api/governance/rule-runs -> 400 Bad RequestAGENTS.md,docs/agents/product-operability-and-script-contract.md,docs/plansA1 alignment request, and user request to ship then rebuild deployment fromorigin/mainorigin/mainpr-review-agent,agent-governance,CI, plus repository branch protection checks after retarget tomainFrontend Verification
/ui#a1/ local branch verification URLhttp://127.0.0.1:5174/#a1a1-source-local,a1-source-minio,a1-step-pick,a1-step-run,a1-step-issues,a1-step-bcf,a1-trigger-convertfilesTreewith a server-local IFC path, plus MinIO key fixture松洲好宅/root/main/u1/model.ifcifc_source_path; A1 conversion button hands off to#conv; BCF review panel preserves existing topics on idempotent retryorigin/main, then run A1 page demo/test against deployed UIValidation
npm test -- src/console/console.test.tsx src/console/A1ViewerEmbed.test.tsx src/console/incomingHandoff.test.tsx src/console/governanceClient.test.ts— 4 files / 120 tests passed.npm run build— Vite build completed; only existing chunk-size warning.git diff --check.detect_changes: MEDIUM scope, changed files limited to the A1 console surface and tests.npx tsc --noEmit -p tsconfig.jsonstill fails onsrc/console/windowParentMessage.dom.test.tsx(292,61): error TS6133: 'init' is declared but its value is never read.Deployment Plan After Merge
After this PR is merged into
main, run./scripts/dev/rebuild-test-deploy.ps1 -Buildfrom the repository root. This helper must freshly fetchorigin/main, reset/rebuildD:\Users\deploy\AI-bim-geo, and execute./scripts/deploy.ps1 -Buildfrom the deployment checkout. Then verify A1 in the browser from the deployed UI.Summary by CodeRabbit
New Features
Bug Fixes