docs(space): 空间配置改版设计与 P1 实现计划 --story=132989396 - #782
Conversation
新增空间配置改版设计文档(可配置/可解释/可验证/可扩展)与 P1 分步实现计划: 后端声明式元数据框架 + 验证接口骨架 + 前端双栏配置中心 + 通用控件, 复杂配置项本期 JSON 兜底不回退,凭证联动与 API 插件实时预览在 P2/P3 单独成文。 Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
审查总结
本 PR 为纯文档变更,新增空间配置改版的设计文档和 P1 实现计划,文档放置和命名均符合 docs-management.mdc 规范。
整体评价: 设计文档结构清晰,方案论证充分(选型对比、向后兼容、分期规划),实现计划遵循 TDD 流程,与设计文档交叉引用正确。以下为几点小建议,供实现时参考:
✨ 改进建议(Minor)
-
Vue watch 写法(计划 Task 6 ConfigDetail.vue):
watch: { config: { handler(val) {...} } }未配置deep或immediate,按项目 Vue 规范应简写为函数形式config(val) { ... }。 -
verify view 异常捕获(计划 Task 3):
get_config处使用except Exception过于宽泛,建议捕获具体异常(如KeyError),避免掩盖非预期错误。 -
运行命令中的本地路径:计划中
Run:均使用绝对路径/Users/dengyh/...,AI agent 实现时需自行替换为工作目录相对路径,建议在计划顶部统一声明为变量或使用cd <worktree-root>占位。
无 Critical 或 Important 级别问题。文档质量良好,可合入。
面向视觉设计师的交付稿,基于已确认的设计文档与 P1 计划产出: - screens/*.md:5 屏 wiremd 低保真线框源(可 diff / 可导 Figma) - html/*.html:渲染后的自包含 HTML,右侧带编号交互标注 - shots/*.png:全页截图 - README.md:概览 + 3 张 Mermaid 流程图(导航流 / 验证状态机 / 引用型控件联动)+ 每屏编号交互说明表
There was a problem hiding this comment.
增量审查总结
本次推送新增 prototypes/output/space-config-redesign/ 目录,包含 wiremd 低保真线框源文件、渲染 HTML 和截图,用于交付给视觉设计师。
整体评价: 纯原型/设计物料新增,无源代码变更,无安全或性能风险。原型结构清晰(README + screens + html + shots),交互标注与设计文档 docs/specs/2026-07-06-space-config-redesign-design.md 对应良好。prototypes/output/ 目录在仓库中已有先例(123dd66),放置合理。
前次审查建议状态
前次审查的 3 条 Minor 建议(Vue watch 简写、except Exception 范围、计划中本地路径)均针对 docs/plans/2026-07-06-space-config-redesign.md,该文件在本次推送中未修改,建议在实现阶段落实时处理。
本次新增内容
无 Critical 或 Important 级别问题。原型交付物料质量良好,可合入。
- README 去掉面向读者身份的交付/说明性文案,聚焦原型内容本身 - 移除 html/(与 shots 截图同内容),需要交互查看时按 README 命令用 screens/*.md 现渲染
There was a problem hiding this comment.
增量审查总结
本次推送(57e58d8)精简了原型 README 说明文案,并移除了 5 个生成的 HTML 文件(~4159 行),保留 wiremd 源文件和截图作为单一事实来源。
整体评价: 纯文档/原型物料清理,变更方向合理——生成产物不入库、需要时从 screens/*.md 现渲染,减少仓库膨胀和 diff 噪音。无源代码变更,无 Critical / Important 级别问题。
✅ 改进确认
- 移除生成 HTML 是好的实践,README 中已保留渲染命令供需要时使用
- 截图总计 ~1MB,体积可控
- 文案精简后信息密度更高,去掉了面向"交付设计师"的冗余说明
无新增问题需要报告。
There was a problem hiding this comment.
增量审查总结
本次推送(f2e2299)新增 prototypes/output/space-config-redesign/designs/ 目录下 5 张蓝鲸风格简易设计稿 PNG(含红色交互标注),并在原型 README 目录表中补充了 designs/*.png 行。
整体评价: 纯设计物料新增,无源代码变更,无 Critical / Important 级别问题。
✨ 改进建议(Minor)
- 设计稿文件体积偏大:每张 PNG 约 1.3 MB(5 张共 ~6.5 MB),相比
shots/目录的低保真截图(~200 KB/张)大了约 6 倍。由于项目未启用 Git LFS,这些二进制文件会永久膨胀仓库体积。建议在合入前考虑:- 用
pngquant/oxipng等工具做无感知压缩(通常可减 40-60%),或 - 若后续还会频繁迭代设计稿,考虑对
prototypes/**/*.png配置 Git LFS
- 用
以上仅为建议,不阻塞合入。
基于 wiremd 低保真原型产出 5 屏高保真简易设计稿(含红色交互标注), 顶栏用户与空间选择器使用 admin / 演示空间 (100) 占位,避免内部名称泄露。 Co-authored-by: Cursor <cursoragent@cursor.com>
d2bd6d7 to
6f3b9be
Compare
There was a problem hiding this comment.
增量审查总结
本次推送(6f3b9be)将设计稿新增与脱敏合并为一个提交,最终内容为 prototypes/output/space-config-redesign/designs/ 下 5 张蓝鲸风格设计稿 PNG 及 README 目录表补行。
整体评价: 纯设计物料变更,无源代码改动,无 Critical / Important 级别问题。
前次审查建议状态
| 建议 | 状态 |
|---|---|
| Vue watch 简写(Task 6 ConfigDetail.vue) | 待实现阶段落实 |
except Exception 范围过宽(Task 3 verify view) |
待实现阶段落实 |
| 计划中本地绝对路径 | 待实现阶段落实 |
| 设计稿 PNG 体积偏大(~1.3 MB/张) | 未处理,建议合入前用 pngquant/oxipng 压缩或配置 LFS |
本次变更
- ✅ 脱敏处理已合并入设计稿提交,设计稿中不再暴露内部信息
- ✨ README 目录表新增
designs/*.png行,索引完整
无新增问题需要报告。
新增 docs/plans/2026-07-10-space-config-redesign-p2-p3.md:在 P1 元数据框架 基础上,覆盖 credential_map / api_plugin_config(含 uniform_api 实时预览验证)/ plugin_scope / engine_kv 复合控件、结构化↔JSON 源码切换与 canvas_mode 图示补图, 按 TDD 分步给出可执行代码;P1 plan 后续计划段落回链本文件。 Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
增量审查总结
本次推送(a72af44d)新增 docs/plans/2026-07-10-space-config-redesign-p2-p3.md(P2/P3 实现计划,1484 行),并在 P1 计划中补充了交叉引用。
整体评价: 纯文档变更,无源代码改动,无 Critical / Important 级别问题。P2/P3 计划结构与 P1 一致(TDD 流程、前后端分步),与设计文档 §5–§9 的覆盖对应关系清晰(Self-Review 已自查)。技术实现细节验证通过——UniformAPIClient、UniformAPIConfigHandler、Credential、ApiModel.get() 等引用的类名与路径均与现有代码一致。
前次审查建议状态
| 建议 | 状态 |
|---|---|
| Vue watch 简写 | P2/P3 中新增的 watch: { value: { handler, immediate: true } } 因使用了 immediate 属于合理用法,无需改动 |
except Exception 范围 |
P2 Task 1 的 verify 实现中对 UniformAPIClient 调用结果用 request_result.result 判断而非异常捕获,设计合理 |
本地路径 /Users/dengyh/... |
同 P1,仍在 Run: 命令中使用,建议实现时替换(已在前次审查标记,不重复报告) |
✨ 改进建议(Minor)
api_categories可选但ApiModel定义为必填:P2/P3 计划 Task 5 的ApiPluginConfig.vue中api_categories标注为"可选",但现有ApiModel中api_categories: str是必填字段(非 Optional)。实现时verify()对缺少api_categories的配置可能因 pydantic 校验失败而报错——建议实现阶段确认是否需将ApiModel.api_categories改为Optional[str]。
无新增 Critical / Important 级别问题。文档质量良好,可合入。
- common(exclude_none_fields / enable_api_parameter_conversion)从 api_key 分块中移出,独立为「通用设置」区,明确其对全部 api_key 生效 - 测试按钮下沉为每个 api_key 各自的「测试该 api_key」,对齐后端 verify 的 api_key 粒度;验证预览区标注「当前测试:bkci」 - 同步更新 wiremd 源 screens/04 与 README 交互说明表 Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
增量审查总结
本次推送(4545661)调整 prototypes/output/space-config-redesign/ 下 API 插件配置屏(屏 4)的设计:
- common 独立成区 — 将
exclude_none_fields、enable_api_parameter_conversion等跨全部 api_key 的通用开关从原先的 api_key 分块中抽出,独立为顶部区域,语义更清晰。 - 按 api_key 粒度测试 — 验证按钮从全局「测试连接」改为每个 api_key 各自带「测试该 api_key」按钮,预览区标注当前被测 api_key,便于精确定位接入异常。
整体评价: 纯原型/设计物料调整(3 个文件:README 交互表格 + wiremd 源文件 + 设计稿 PNG),无源代码变更,无 Critical / Important 级别问题。设计改动方向合理——common 开关独立成区避免歧义,按 api_key 粒度验证与后端 verify 接口的设计(P2/P3 计划中以单个 api_key 为单位调用)保持一致。
前次审查建议状态
前次审查均为纯文档/物料新增的 Minor 建议,本次无新发现需要报告的问题。
There was a problem hiding this comment.
审查总结
本 PR 为纯文档/设计物料变更(设计文档 + P1/P2/P3 实现计划 + 低保真原型线框与设计稿),无源代码改动。
文档规范检查:
docs/specs/2026-07-06-space-config-redesign-design.md— 命名格式YYYY-MM-DD-<topic>-design.md✅docs/plans/2026-07-06-space-config-redesign.md/2026-07-10-space-config-redesign-p2-p3.md— 命名格式YYYY-MM-DD-<feature-name>.md✅- 计划中引用 spec 路径均指向
docs/specs/✅ prototypes/output/已有仓库先例,无问题
前次审查建议追踪:
| 建议 | 状态 |
|---|---|
| Vue watch 简写(ConfigDetail.vue 计划) | ⏳ 待实现阶段落实 |
except Exception 宽泛捕获(verify action 计划) |
⏳ 待实现阶段细化异常类型 |
| 设计稿 PNG 体积偏大(~1.3 MB/张) | ℹ️ 已知,可接受 |
无 Critical / Important 级别问题,无新增行内评论。
* feat: 空间管理空间配置改版 --story=132989396 # Reviewed, transaction id: 83598 * feat: 空间管理空间配置改版 --story=132989396 # Reviewed, transaction id: 83604
There was a problem hiding this comment.
🔍 Incremental Code Review — Space Config Redesign (Source Code)
Scope: This review covers the new source code pushed in the latest synchronize event (frontend Vue components, store action, i18n, CSS changes). Documentation/plans/prototypes are not re-reviewed.
📋 Summary
This is a significant frontend feature implementing the Space Config Redesign (dual-pane master-detail UI with metadata-driven control registry). The architecture is clean and follows existing project patterns (Vuex store actions, bk-magic-vue components, i18n). Key issues found are around error handling robustness, missing prop validation, and a potential reactivity issue.
🔑 Key Findings
| # | Severity | File | Issue |
|---|---|---|---|
| 1 | ConfigDetail.vue |
handleVerify and handleSave share duplicated error handling pattern — but more critically, resp.data || resp assumes specific response shape without null-guard |
|
| 2 | ApiPluginConfig.vue |
Mutating prop-derived reactive apiItem.testLoading/testResult directly on array items can miss reactivity in edge cases (Vue 2 caveats) |
|
| 3 | CredentialMap.vue |
response.data.results.filter(...) — no null-check on response.data.results; API failure could throw TypeError |
|
| 4 | 💡 Minor | controls/index.js |
select maps to OptionRadio — semantically misleading; should map to a dropdown select or be documented |
| 5 | 💡 Minor | ConfigDetail.vue |
CSS nesting issue: .detail-help closing brace misaligned (line ~3616), may cause style leakage |
| 6 | 💡 Minor | EngineKv.vue |
isValueEqual uses JSON.stringify for comparison — breaks on key order differences |
| 7 | 💡 Minor | MemberSelectorControl.vue |
Syntax: extra } on props definition (line 5108 area) — default: () => ({}) followed by stray }, |
✅ Positives
- Clean control registry pattern with fallback to JSON editor — extensible and safe
- Consistent
model: { prop: 'value', event: 'change' }across all controls — good v-model compliance - Proper
validate()method exposed on composite controls for parent form integration - Dual-mode (structured ↔ JSON) with JSON parse validation before switching — good UX
- i18n coverage is comprehensive (90+ keys in both cn/en)
- Rewritten
index.vueproperly cleans up the old table-based approach
📝 Inline Comments Below (6 items)
There was a problem hiding this comment.
ConfigDetail.vue:168)
resp.data || resp in handleVerify will throw if the API returns an unexpected shape. Consider:
this.localVerifyResult = (resp && resp.data) ? resp.data : (resp || { ok: false, error: { message: 'Empty response' } });Also, handleVerify and onControlVerify are nearly identical — consider extracting a shared helper.
There was a problem hiding this comment.
ApiPluginConfig.vue ~ testApi method)
In testApi, assigning apiItem.testResult = { ok, error, data } replaces the property value. Since testResult is initialized as null in addApi()/parseValue(), Vue 2's reactivity system tracks it correctly for the initial render cycle. However, after parseValue re-creates items (triggered by external prop changes), the newly created objects get fresh observers — so this is safe.
Still, for defensive consistency with the rest of the codebase, prefer this.$set(apiItem, 'testResult', {...}) to guarantee reactivity in all paths.
There was a problem hiding this comment.
CredentialMap.vue:190)
response.data.results.filter(...) — if the API returns an unexpected shape where response.data is null/undefined, this throws TypeError. The catch block only sets credentialList = [], but the error happens before reaching it since it's not an async rejection but a sync TypeError.
Suggested fix:
const results = (response.data && response.data.results) || [];
this.credentialList = results.filter(item => item.type === 'BK_APP');There was a problem hiding this comment.
💡 Minor — Semantic mismatch: select → OptionRadio (controls/index.js:14)
The registry maps select control type to OptionRadio, which renders as radio buttons. If a backend config declares ui.control = 'select', users might expect a <bk-select> dropdown.
If intentional (treating select as radio for limited-option fields), add a brief comment explaining why. Otherwise, consider a simple OptionSelect.vue wrapper.
There was a problem hiding this comment.
💡 Minor — JSON.stringify equality is order-sensitive (EngineKv.vue:89)
JSON.stringify(a) === JSON.stringify(b) returns false for objects with same keys/values but different order. This could cause unnecessary re-parses when the parent emits back the same semantic value.
The project already has tools.isDataEqual (used in CredentialMap.vue). Suggest using it here for consistency:
import tools from '@/utils/tools.js';
// ...
isValueEqual(a, b) { return tools.isDataEqual(a, b); }There was a problem hiding this comment.
💡 Minor — Formatting issue in props (MemberSelectorControl.vue:17)
The schema prop has an unusual brace placement:
schema: {
type: Object,
default: () => ({}) }, // closing brace on same line as default
},Should be:
schema: {
type: Object,
default: () => ({}),
},Not a runtime bug, but inconsistent with the rest of the codebase's formatting.
- 优化api/space/admin/space_config/config_meta接口 增加配置项的group/help/ui/verifiable字段返回 - 增加api/space/admin/space_config/verify 校验前端的数据,目前只支持 uniform_api 字段的校验 # Reviewed, transaction id: 83619
feat: 空间配置优化-后端接口 --story=1070120217132989396
| try: | ||
| config_cls = SpaceConfigHandler.get_config(data["name"]) | ||
| except Exception as e: | ||
| return Response({"ok": False, "error": {"message": str(e)}}) |
| ) | ||
| return Response({"ok": True, "data": verify_data}) | ||
| except SpaceConfigVerifyNotSupported as e: | ||
| return Response({"ok": False, "error": {"message": str(e), "not_supported": True}}) |
| return Response({"ok": False, "error": {"message": str(e), "not_supported": True}}) | ||
| except Exception as e: | ||
| logger.error(f"[space_config verify] name={data['name']} error: {e}") | ||
| return Response({"ok": False, "error": {"message": str(e)}}) |
There was a problem hiding this comment.
增量审查总结
本次推送(55bb70e)新增 UniformApiConfig.verify() 方法(~140 行,包含三轮外部 HTTP 调用)和对应的 SpaceConfigAdminViewSet.verify action,以及完善的单元测试。整体实现与设计文档一致,测试覆盖全面。
以下为发现的问题:
⚠️ 逻辑(Important)
- Off-by-one:
configs.py:524—if len(samples) > 5: break实际允许收集 6 个样例(len 达到 6 时才 break),与注释"最多 5 个"不一致,应改为>= 5。
✨ 改进(Minor)
-
魔法数字:
configs.py:494/524—limit: 50和> 5建议抽为类级常量(如VERIFY_LIST_LIMIT = 50、VERIFY_MAX_SAMPLES = 5),便于维护和调整。 -
categories 循环无上界:
configs.py:500—for category in categories每次迭代发一次 HTTP 请求,若网关分类较多(数十个)可能导致验证接口响应超慢。建议加截断(如categories[:10])或在拿到足够 api_list 后提前 break。
# Reviewed, transaction id: 83924
feat: 空间配置优化-后端接口,修复Bug --story=1070120217132989396
There was a problem hiding this comment.
增量审查总结
本次推送(e08fffd, merged via c7f4c2d)修复了 UniformApiConfig.verify() 方法中获取凭证名称的 Bug:当 ApiGatewayCredentialConfig 存储多 scope 的字典格式时,缺少 scope='default' 参数会导致 get_value() 返回 None,从而误报「空间未配置默认网关凭证」。
修复方式正确:ApiGatewayCredentialConfig.get_value() 在 scope 非空时会走 config.get(scope) or config.get('default') 逻辑,传入 scope='default' 确保字典格式下能正确提取凭证名。
整体评价: 单行 Bug 修复,逻辑清晰,无 Critical / Important / Minor 级别新问题。
前次审查建议状态
| # | 问题 | 状态 |
|---|---|---|
| 1 | Off-by-one: len(samples) > 5 应为 >= 5(L524) |
⏳ 未修复(不在本次变更范围) |
| 2 | 魔法数字 limit: 50 / > 5 建议抽常量 |
⏳ 未修复(不在本次变更范围) |
| 3 | categories 循环无上界 | ⏳ 未修复(不在本次变更范围) |
无新增行内评论。
* fix: 空间配置api配置测试接口结构修改 --story=132989396 # Reviewed, transaction id: 83932 * fix: 空间配置api配置测试接口结构修改 --story=132989396 # Reviewed, transaction id: 83934
There was a problem hiding this comment.
##增量审查总结
本次推送(2b2d3ea)修复了空间配置验证接口的前端响应解析问题,包含三处变更:
- Store 层修复(
spaceConfig.js:46):response.data→response.data.data,正确解包DRFSimpleGenericViewSet的{result, data, code, message}信封结构,使返回值与ApiPluginConfig消费端resp.ok/resp.data的预期一致。 - 移除死代码(
ConfigDetail.vue):删除handleVerify方法,其唯一引用在注释模板中,无运行时影响。 - UI 优化(
ApiPluginConfig.vue):将测试失败冒号分离为独立<span>并加margin-right: 4px,改善排版。
整体评价: 修复方向正确,变更最小化,无Critical / Important 级别问题。
前次审查建议状态
| # | 问题 | 状态 |
|---|---|---|
| 1 | Off-by-one: len(samples) > 5 应为 >= 5(L524) |
⏳ 未修复(不在本次变更范围) |
| 2 | 魔法数字 limit: 50 / > 5 建议抽常量 |
⏳ 未修复(不在本次变更范围) |
| 3 | categories 循环无上界 | ⏳ 未修复(不在本次变更范围) |
✨ 改进建议(Minor)
ConfigDetail.vue:348 — this.localVerifyResult = resp.data || resp:store 修复后 resp 已是 {ok, data, error} 结构,resp.data 会取到内层 verify_data丢失 ok/error 字段。当前无控件触发此路径(@verify 事件无人emit),但建议改为 this.localVerifyResult = resp 以避免后续接入新控件时踩坑。
# Reviewed, transaction id: 83953
feat: 空间配置优化-后端接口,修复新建流程获取api插件的Bug --story=1070120217132989396
There was a problem hiding this comment.
增量审查总结
本次推送(e6ff31d, merged via a5593df)修复了 _get_api_credential 函数中与verify() 同类的 scope 默认值 Bug:当模板/任务没有 scope_type 和 scope_value 时,将 scope 从 None 改为 "default",使 ApiGatewayCredentialConfig.get_value() 能正确走 config.get("default") 分支获取默认凭证。
修复正确且一致:与 e08fffd 中对 verify() 的修复同理,get_value() 的 if scope: 分支要求非空 scope 才会查询字典配置,传 None 会导致直接返回 None 而误报凭证不存在。
无Critical / Important / Minor 级别新问题。
新增空间配置改版设计文档(可配置/可解释/可验证/可扩展)与 P1 分步实现计划:
后端声明式元数据框架 + 验证接口骨架 + 前端双栏配置中心 + 通用控件,
复杂配置项本期 JSON 兜底不回退,凭证联动与 API 插件实时预览在 P2/P3 单独成文。