CODE REVIEW · STORE / MR 923
Request changes · 修改后再合并学校膳食营养生产闭环
详细代码审查
源版本 622dafe3e · 115个文件 · 2026年9月28日
审查日期:2026-09-28。结论:Request changes,建议修复后再合并。
发现 11 项已确认问题(5 项 P1、6 项 P2)和 1 项依赖目标数据库结构的发布风险。结论基于源码、真实需求和隔离复现,不代表生产事故已发生。未改业务代码,未发布云效评论、提交审批、合并 MR 或访问生产。
1. 审查版本与契约
- MR 923:#VWCG-1366 实现学校膳食营养生产闭环。状态 UNDER_REVIEW;描述为空;平台未通过项为 reviewer approved,非已证实构建失败。
- 源分支
develop_nutrition_0918,HEAD622dafe3ec2fd237d12e6aa127e87d87baa77940。 - 目标分支
release,HEAD3f8dc764a85446094001726de7fe92f8e761ee3f。 - merge-base
4e921ab640d56514d97b4f1471f271b0baea4281;115 文件,新增 26693 行、删除 34 行;源分支领先 23 个提交、落后 18 个提交。按 merge-base→源 HEAD 审查,未把目标分支独有变更当作本 MR 删除。 - 通过当前用户个人 Token 的云效 OpenAPI 读取 VWCG-1366~1374 九条需求。规范依据为读取时的真实正文;历史产品笔记中“没有学生餐”的限制已被本次明确学校膳食需求覆盖。
- 任务契约:只读评审整个 MR,按可达性、业务影响、证据强度排序;输出位置、触发、实际/期望、修复和回归。开源项目只提供工程参考,不替代本项目业务规则。
- 执行方式:初始常规 Codex;发现跨权限/发布/采购契约后按 task-execution-mode-router 和 ruflo-isolated-task-harness 拆分三个只读审查角色。
ruflo命令不可用,降级常规 Codex 执行,保留角色边界和验证,不声称启动过 Ruflo runtime。 - 使用技能:
cli-anything入口识别后转正式代码审查;code-review-and-quality、karpathy-guidelines、yunxiao-cli-interaction、yunxiao-codeup-pr-association、vue-best-practices、zhct-docker-php-verification;开源取证走agent-reach的 GitHub CLI 后端。
2. 问题总表
路径均相对于上述 HEAD 的 store/,不是本机当前其他开发分支。
| 编号 | 级别 | 问题 | 代码位置 | 证据 |
|---|---|---|---|---|
| R01 | P1 | 必要配方缺失、每人量为0仍可审核发布 | ApprovalService.php:318–321 | 真实服务+隔离SQLite |
| R02 | P1 | 切换采购单后,旧响应把A数量提交给B | purchaseDemand.vue:295–305、352–358 | 真实Vue方法+乱序Promise |
| R03 | P1 | 自动保存失败覆盖未保存输入,仍可分析/切换/提审 | schoolNutrition/index.vue:2536–2539、2665–2668 | 真实Vue方法+失败响应;需求1368 |
| R04 | P1 | 多指标完整时仍全部退回ID排序 | NutritionCandidateRanker.php:117–126 | 纯服务复现;需求1374 |
| R05 | P1 | 部分缺项仍对子集做营养排名,未整组回退 | NutritionCandidateRanker.php:170–173 | 纯服务复现;需求1374 |
| R06 | P2 | 仅新建食谱权限可新增模板和供餐对象 | SchoolNutritionPlan.php:83–87、97–101 | 真实权限函数+菜单夹具 |
| R07 | P2 | 合法特殊规则维护角色被四个操作全部拒绝 | SchoolNutritionPlan.php:135、143、151、170 | 真实权限函数+真实复合filter |
| R08 | P2 | A保存的晚到响应污染当前B的锁版本 | schoolNutrition/index.vue:2531–2535、2654–2656 | 真实Vue方法+延迟响应 |
| R09 | P2 | 首期仅展示的季节条件实际改变推荐排序 | NutritionCandidateRanker.php:86–96 | 纯服务复现;需求1374 |
| R10 | P2 | 推荐结果评价混入夜宵,使完整正餐失去评价 | RecommendationService.php:831–843、863–866 | 真实计算方法复现 |
| R11 | P2 | 推荐已写入后worker中断,重投将任务标记失败 | RecommendationTaskService.php:152–179 | SQLite重建崩溃窗口并重投 |
| D01 | 条件性P1 | 导出菜单create_by为41字符,仓库结构仅32 | db/v2/v2.36.8.sql:5、db/menu.sql:26 | 仓库DDL+字符串长度;未访问客户库 |
P1:直接影响主要业务闭环或已确认算法;P2:有明确触发条件的功能/权限/恢复缺陷。D01 未伪装成所有客户都已发生的失败。
3. 发布与权限
R01:缺必要配方仍然发布
DraftEditorService 允许缺配料或重量的菜保存为草稿,只返回警告。这种草稿容错合理,但 ApprovalService::assertReadyForSubmit() 仅检查对象、有效餐次、菜品和非空哈希,未检查食材标识、每人量、单位。提审和审核都复用它,因此警告一路变成正式发布。
已通过真实 saveDraft → submit → approve 复现:无配料时返回 DISH_INGREDIENT_MISSING,仍 status=3,旧菜单新增1条,采购单食材0条;每人量0时返回 INGREDIENT_WEIGHT_MISSING,仍发布,采购条目甚至 calculation_status=1 且 error_code 为空。
这违反1366/1370/1374中“仅营养成分缺项可以放行,必要食材/用量/单位不得缺失”的边界。应保留草稿保存,在提审与发布共用门禁检查必要配方;营养值缺项仍单独警告。补无配料、非法食材ID、零/负/缺用量、未知单位,以及仅缺营养仍允许发布的正反测试。
R06:新建食谱权限扩大到了设置维护
新建食谱菜单 filter 是 p/schoolnutritionplan/create。继承的 checkFunctionPermission() 将它作为无首尾边界的正则,命中 createtemplate、createtargetgroup;两个新增写端点未做具体动作权限检查。仅有新建食谱权限的非超管可新增全商户模板和本人学校内的供餐对象。
复现直接加载真实权限函数;这是动作越权,没有据此推断跨学校读写或审核越权。应对两个端点增加完整动作权限,或规范化并精确匹配全部菜单路径,同时回归旧接口。
R07:复合filter与精确匹配互不兼容
特殊规则维护菜单将四个路径用 | 放在同一 filter;前置正则检查能通过,但新控制器又调用 requireMenuAccess([单一路径])。其底层精确函数不拆分复合filter,四个操作都拒绝已经合法授权的非超管。
应统一“拆分动作→规范化→精确比较”的权限语义,或将菜单拆成四个动作;不能通过删除新增检查退回前缀漏洞。测试必须使用真实 SQL 菜单字符串以及普通角色。
完整调用链和排除项:发布与权限专项报告(本地留档)。
4. 前端保存与采购
R02:采购数量串到另一张单
loadPreview() 读取发起请求时的 selectedDemandId,但收到响应时直接覆盖当前 items/lockVersion;confirmReview() 再把当前 selectedDemandId 与这些数据组合。选择切换期间仍可确认,且旧响应没有请求序号检查。
复现:先请求A=101,再切B=202;B先返回数量20,A后返回数量10。界面选中B但显示A,实际确认载荷为 demand_id=202, ingredient_id=1, confirmed_quantity=10, expected_version=0。两单食材集合和锁版本相同时,后端合法性校验无法识别界面来源已错配;改量原因满足时具备可接受条件。本轮动态验证到实际前端请求载荷,未连接HTTP后端提交真实采购。
应将 loadedDemandId 与选择、行项目、版本作为同一份状态;loading或两ID不一致时禁止确认;丢弃过期成功和失败响应。补A/B反向返回、请求未完成即确认、加载失败保留旧单等用例。
R03:保存失败被回读覆盖
人数/菜品保存失败时直接调用 loadWorkbench(),重新拉取旧服务端值、关闭编辑界面。失败变更没有独立保留;分析切换、记录选择和提审也未统一检查保存失败/保存中状态。这与1368明确要求的“保留输入、阻止切换分析提审、允许重试”相反。
应维护 pending edits,区分明确失败、冲突和结果未知;结果未知回读只用于核对,不直接覆盖输入;提供重试或明确放弃。补断网、500、版本冲突、保存中离开页面与提审测试。
R08:成功响应也可能污染别的记录
A的保存未返回就切B,回调写的是 this.workbench.version.lock_version。真实方法复现:B服务端版本0,被A的返回值改成2。之后B正常编辑报冲突,继而触发R03丢输入。菜品保存还会污染当前content_hash。
应捕获请求对应的plan_version_id与序号,只更新同一记录的状态;如果允许切换,用按版本ID分区的编辑状态。R03与R08症状相关,但一个是失败处理,一个是成功响应归属,回归必须分别覆盖。
完整位置与动态脚本:前端、导入与SQL专项报告(本地留档)。
5. 推荐算法与真实需求不一致
R04:多指标本可计算,却整体放弃
1374已明确“同指标同方向去重→等权平均名次→稳定ID打破并列”,不是待研发自定复杂权重。当前ranker遇到两个以上指标直接 MULTI_METRIC_POLICY_UNDEFINED,退回ID。
反例:同频、能量均100;菜1蛋白10/铁20,菜2蛋白20/铁10,菜3蛋白30/铁30;两个指标降序。按要求应为 [3,1,2],代码为 [1,2,3],影响前N道实际入选。应实现已确认的排名规则并删除“尚无组合策略”的错误支持说明。
R05:缺项时采用了另一套排序政策
1374要求同类同频组任一候选缺必要指标或有效分母时,整组按频次+稳定ID回退。当前代码把完整候选排完,再将缺项候选放尾部。
反例:菜1缺蛋白、菜2为20、菜3为30。应回退 [1,2,3],实际 [3,2,1],且完整子集仍写 NUTRITION_RANKING。应先验证整个比较组的数据完整性,再决定排序或整体回退,不把缺记录当营养较差。
R09:季节仍参与本期选菜
1374首期规则要求时令只保存展示、不参与排序。显式季节却启用了 seasonalPriority;同频素菜ID1占比0、ID2占比70,修改季节会把 [1,2] 改为 [2,1]。
应保留季节输入但不启用排序,并准确显示“未参与算法”。如果产品决定启用,需要先更新需求及验收,不能由当前实现反向宣布需求已变。
R10:夜宵影响了完整正餐评价
正式 NutritionAnalysisService 已分开正餐和夜宵;推荐的 nutritionTargetEvaluation() 却把夜宵也交给同一个周评价器。午餐650kcal可得到585~715范围;增加夜宵后整体变成 MEAL_SHARE_NOT_DEFINED,正餐参考值消失。
应复用正式分析的范围拆分:正餐计算正餐日均与评价,夜宵/全部供给独立保留统计;仅夜宵日不进入正餐日均分母。这里影响的是推荐结果/推荐留痕,不能扩大成整个营养分析页都有该问题。
测试陷阱: 现有 NutritionCandidateRankerTest 中三条测试分别断言“多指标回退”“完整子集排前”“季节排序”,恰好固化了R04/R05/R09。修复时要先把测试预期与1374对齐,再修改实现。
完整数据、定位和证据:营养、推荐与采购专项报告(本地留档)。
6. 队列恢复与升级风险
R11:已写入草稿,重投后报告失败
RecommendationTaskService::process() 先调用 RecommendationService::apply() 提交草稿与推荐APPLIED记录,再独立更新任务SUCCEEDED。若worker在两者之间终止,持久状态是“草稿已变、任务RUNNING”。重投看到RUNNING继续从preview开始,用原expected_version查询;已提交的新版本导致 PLAN_VERSION_CONFLICT,随后标FAILED。
隔离SQLite重建这一崩溃窗口再重投,实际为:after_redelivery=FAILED、failure_code=PLAN_VERSION_CONFLICT,同时已有35条菜品、1条APPLIED推荐。反例验证的是持久状态恢复,不声称已进行了真实进程kill或Redis故障注入。
修复时将任务最终状态与业务写入关联为可原子提交的结果,或重试首先按任务确定性request_id回读APPLIED推荐和实际版本,恢复成功结果。另需明确RUNNING持有者/租约,避免重复投递并发执行;这个并发点列入补测,不额外计为已复现缺陷。
D01:新增菜单字段超长
db/v2/v2.36.8.sql:5及db/menu.sql:26写入的create_by是 2026-09-28-school-nutrition-weekly-export,共41字符。仓库 db/zhct_jx206.sql:30521、db/zhct_ali_bailian.sql:1861 的同列为varchar(32),本MR未提供扩容迁移。
若目标库沿用该结构并启用严格模式,插入会失败;非严格模式会截断。这是基于仓库DDL的条件性发布风险,未声称读过客户现网结构。最小修复是使用32字符以内的标识;发布前在隔离MySQL按实际sql_mode、schema执行升级并验证权限菜单。
7. 开源实现对照
本次实际通过Agent Reach的GitHub CLI读取以下源文件,并冻结SHA。没有安装这些框架,也不建议为修复本MR整体迁移技术栈。
| 项目与冻结版本 | 已读取证据 | 可借鉴到本MR的最小做法 |
|---|---|---|
Laravel Framework 7d104b24974f4eb7140488c446712d9c31fb346e |
WithoutOverlapping.php | 任务执行先取得带有效期的锁,finally释放,拿不到锁则延迟重投;用于R11的并发执行边界。锁本身不解决业务写入与任务状态分开提交,仍需事务或结果回读。 |
Vue Router 3.6.5 4f934f712b0ce333419d475254e860f81d7014cd |
navigation-guards.md | 用beforeRouteLeave处理未保存内容离开;本页面内部换食谱、换分析tab也须经过同一保存门禁。用于R03/R08,不能只补浏览器路由守卫。 |
PHP-Casbin c856b5f9a009af6bc4679095a2095846a7629470 |
basic_model.conf、rbac_model.conf | 明确主体、对象、动作;RBAC仅负责角色关系,对象/动作保持精确比较。借此统一R06/R07的动作语义,保留学校/档口数据范围校验。 |
同时核对了Laravel队列官方说明和PHP PDO事务说明。开源建议是工程对照;营养排名、时令、夜宵和学校供餐规则必须以当前云效契约及专业数据为准。
8. 验证结果与未覆盖边界
| 检查 | 结果 | 限度 |
|---|---|---|
| MR及9条云效需求 | OpenAPI读取成功;Git源/目标/merge-base冻结 | 反映2026-09-28读取时版本 |
| 新增/改动PHP语法 | 90文件通过 | PHP8.2语法检查,不等同PHP7.3运行验收 |
| 前端学校营养Jest | 4组、21条通过 | 不能证明乱序响应、真实浏览器和HTTP集成正确 |
| 后端18组独立测试 | 105条、1424断言通过 | 其中2组含断言API弃用warning |
| 后端草稿流程选定测试 | 37条、270断言通过 | 合计142条、1694断言;PHP8.2/PHPUnit9.6兼容验证 |
| 发布缺配方反例 | 2条、4断言证实缺陷 | 真实service,隔离SQLite |
| 队列恢复反例 | 1条、5断言证实缺陷 | 持久状态窗口重建,未启动真实worker |
| 权限反例 | 两端越权+四端拒绝均复现 | 真实权限方法,账号/菜单为隔离夹具 |
| Vue乱序/失败反例 | 串单、版本污染、失败重载复现 | 调用源文件方法,Promise及HTTP响应为隔离桩 |
| 营养计算反例 | 多指标、缺项、季节、夜宵复现 | 纯服务,无真实数据访问 |
| 原项目PHPUnit7全套 | 未完成 | PHP8保留字Match导致原runner无法解析;Docker未启动 |
| PHPUnit9学校营养全套 | 未完成 | PHPExcel大括号数组语法与PHP8不兼容;未把环境问题记作本MR缺陷 |
| 生产运行时/集成 | 未运行 | PHP7.3、真实MySQL升级/锁竞争、Redis重投、UI浏览器E2E、硬件同步与全量构建待补 |
辅助bootstrap仅忽略PHP8对旧框架的deprecated诊断,不修改业务/框架源码。所有SQLite运行目录在 隔离临时目录;未复制生产凭据。测试“通过”只是当前断言通过,R04/R05/R09已指出其业务预期错误。
已核对正常机制:审核使用事务;CAS失败回滚;发布重新检查对象状态;特殊供餐证据绑定内容/规则哈希;旧菜单多个写入口有scope guard;已发布历史按快照读取;基础采购q×人数/1000换算未发现独立错误。未将这些误报为缺失。
未升格为确定缺陷:学校与档口真实归属映射、多份推荐主档source_hash完整性、发布与采购交接并发提醒、导出再导入的serving_factor往返损失、UI缺每人克重编辑。这些应补定向验收,不能直接据静态疑点宣称已经造成事故。
异步导出联动:这次新增的是学校食谱/采购专用导出;未直接改既有九类经营/消费报表查询,本轮未发现必须同步修改它们的证据。新增同步Excel仍须在PHP7.3环境完成生成与回读测试。
9. 修复顺序与验收
- 数据与发布:先修R01/R02/R03,分别证明缺必要配方被拦、采购不串单、保存失败不丢输入。
- 算法契约:按1374重写R04/R05/R09的验收断言,补多方向组合、缺项组、季节不影响输出;同步修R10夜宵范围。
- 权限与异步恢复:R06/R07用真实菜单普通角色跑允许/拒绝矩阵;R08/R11跑反向响应和已提交后重投。
- 目标环境:核实D01字段长度;PHP7.3完整后端测试、前端lint/build、导入导出往返、MySQL事务故障/并发、Redis worker重载后回归。
- 合并验收:在最新release合并结果上重跑,不以当前分支142条测试通过替代合并结果与业务验收。任何P1未闭环时建议保持Request changes。
10. 审查范围说明
本报告对应源版本622dafe3e,仅记录代码审查和隔离验证结果。未修改业务代码,未合并MR;完整测试日志和复现脚本保留在本地证据包。外链不包含凭据、生产数据或本机绝对路径。