Skip to content

完善库存链路与CommonShelf能力#23

Merged
hzb666 merged 25 commits intomainfrom
develop
Apr 1, 2026
Merged

完善库存链路与CommonShelf能力#23
hzb666 merged 25 commits intomainfrom
develop

Conversation

@hzb666
Copy link
Copy Markdown
Owner

@hzb666 hzb666 commented Mar 31, 2026

概要

本次 PR 将 develop 分支合并到 main,主要包含库存链路、操作日志、SSE、前端存储重构,以及 CommonShelf / CAS 主数据能力的完整接入。

主要变更

后端

  • 新增 CommonShelfchemical_name_map 及相关日志模型
  • 实现 CommonShelf 与 CAS 主数据管理接口
  • 调整库存、订单到货、入库、日志记录相关流程
  • 重构部分查询、内部编码、货架生成与导出逻辑
  • 完善操作日志与导入链路
  • 增加缓存失效与部分认证边界整理

前端

  • 调整库存、试剂订单、Dashboard 到货录入相关交互
  • 收敛表单配置、表格列配置、校验逻辑和部分公共组件
  • 更新 README 与部分 wiki 文档
  • 增强浏览器扩展中的购物车导入与订单类型识别

影响范围

  • app/api/**
  • app/models/**
  • app/services/**
  • frontend/src/**
  • browser-extension/**
  • docker/**

测试

已完成:

  • frontend/npm run lint
  • frontend/npm run build
  • ruff check app/

风险点

  • 变更面较大,涉及后端接口、前端页面、导出、日志、SSE、认证边界
  • 建议重点回归:
    • CommonShelf 手动添加、编辑、加瓶、扣减、导出
    • chemical_name_map 新增/编辑/缺失补录续提交流程
    • 试剂订单到货录入位置
    • 库存导出与日志展示
    • Dashboard 和订单相关 SSE 刷新

Summary by cubic

完整接入 CommonShelf 与 CAS 主数据,重构库存到货/入库链路,并新增统一操作日志,提升可追溯性与易用性;同时完善 FTS 参数名、请求头读取、数字输入 onChange 与 SQLite RETURNING 兼容等细节,并在登录密码输入增加大写锁提示。同步引入版本化缓存失效、导入预览确认两阶段、SSE 稳定性优化、浏览器扩展升级与相关文档同步。

  • New Features

    • CommonShelf + CAS:新增 CommonShelfchemical_name_map 模型与接口,支持分组增删、合并、加瓶/扣减、导出与 FTS 搜索;补充纯度字段。
    • 操作日志:新增库存/试剂单/耗材单/CommonShelf/用户多类操作日志表与记录器,开放管理员查询 API;提供日志归档脚本;修正 common_shelf_operation_log.operator_id 外键为 SET NULL,避免删用户误删审计历史。
    • 导入链路:库存导入改为“预览→确认”两阶段(Redis 会话,TTL),限流与冲突重试,改进 Excel 解析与导出结构。
    • 鉴权与会话:统一会话缓存与校验,新增密码修改/重置限流;引入版本化缓存失效,变更时自动撤销全部会话并要求重登。
    • SSE:支持每标签页 client-id、可靠本地/跨进程广播与房间订阅;事件端点简化。
    • 前端与构建:重构本地存储与鉴权骨架屏;CommonShelf UI 与 CAS 管理接入;全站接入 SSE 静默刷新;新增静态库生成脚本与不可索引设置(含 robots.txt);登录密码输入新增大写锁提示;同步相关 wiki 文档。
    • 浏览器扩展:新增购物车标签页优选与订单类型自动识别;站点配置与 URL 构建统一;新增 webRequest 权限。
    • 部署:前端镜像改用 Alpine + nginx Brotli,长效缓存 /lib/assets;更新 docker/nginx 配置。
    • 修复与细化:统一 FTS 绑定参数名避免冲突;Axios 响应头大小写兼容读取;修正数字输入 onChange 取值;为低版本 SQLite 增加 DELETE/UPDATE RETURNING 兼容路径,新增兼容助手 app/core/db_compat.py 并替换所有调用点。
  • Migration

    • 新增环境变量 CACHE_VERSION。部署后如需强制清理前后端缓存并让所有用户重新登录,请递增该值(留空时回退到 APP_VERSION)。
    • 新表会自动创建,无需手动迁移;重启服务即可。
    • 未使用提供的 Docker 镜像时,请同步更新 docker/nginx/nginx.confdefault.conf,开启 Brotli 与静态资源长缓存。
    • 建议升级浏览器扩展到最新版本以使用购物车导入与类型识别能力。

Written for commit 4af4f70. Summary will update on new commits.

hzb666 added 20 commits March 28, 2026 05:45
- 新增浏览器扩展购物车标签页选择逻辑(cart-tab-selection.js)
- 添加订单类型自动检测功能
- 新增站点配置工具函数,统一 URL 构建逻辑
- 重构前端购物车导入页面,将模型、控制器拆分到独立文件
- 统一 SessionCacheIdentity 缓存边界
- 简化 SSE 控制消息解析分支
- 精简静态路径清洗校验流程
- 整理日志聚合、导入和外部查询的辅助代码
- 统一会话缓存同步和 SSE 订阅参数
- 简化数据库、缓存和拼音工具的参数处理
- 新增 CACHE_VERSION 配置
- 后端在版本变化时清理 Redis 前缀缓存
- 后端在版本变化时撤销全部用户会话
- 前端启动前探测 cache_version 并清理本地缓存
  - 保持业务数据与数据库结构不受影响
- 新增鉴权骨架屏组件,仪表盘、表格页、导入页三个样式
- 接入sse,实现静默更新或提醒更新的不同逻辑
- 完善stalebanner组件
- 新增库存申购用户操作日志模型与服务
- 接入用户订单库存关键路径日志写入
- 用户日志聚合改读日志表并补计数查询
- 导入改为预览确认两阶段并记录入库日志
- 会话流程改为单事务提交后执行副作用
- 新增日志归档脚本并补日志查询索引
- 导入改为预览与确认两阶段流程
- 操作日志页扩展多类型展示与详情渲染
- 常用货架搜索高亮与别名展示优化
- 接入版本化静态资源与生成脚本测试
- 调整前端构建压缩与资源引用配置
- 前端镜像改为 Alpine 并启用 Brotli 模块
- 增加 nginx 主配置并调整站点配置
- 补充静态资源缓存与编码协商头
- 调整忽略规则以纳入前端脚本目录
- 移除重复与冗余的技术徽章展示
- 保留核心运行环境与许可证标识
- 新增common_shelf数据模型
- 新增chemical_name_map模型
- 调整模型导出与数据库初始化
- 删除旧common搜索相关服务
- 新增CAS主数据管理接口
- 重写CommonShelf核心接口
- 调整订单到货与入库相关流程
- 复用并整理货架生成查询服务
- 补齐操作日志与接口挂载
- 调整库存导出字段结构
- 新增CommonShelf导出能力
- 补齐纯度备注相关日志字段
- 同步导出服务实现
- 重构CommonShelf页面与弹窗结构
- 接入CAS主数据管理与补录流程
- 调整表单配置校验与表格列
- 同步日志展示与到货录入交互
- 更新前端依赖与静态资源脚本
Copilot AI review requested due to automatic review settings March 31, 2026 19:23
@deepsource-io
Copy link
Copy Markdown

deepsource-io bot commented Mar 31, 2026

DeepSource Code Review

We reviewed changes in e95ba09...4af4f70 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
JavaScript Apr 1, 2026 12:55p.m. Review ↗
Python Apr 1, 2026 12:55p.m. Review ↗
Secrets Apr 1, 2026 12:55p.m. Review ↗

@kilo-code-bot
Copy link
Copy Markdown

kilo-code-bot bot commented Mar 31, 2026

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Incremental Review

Changes since last review (commit babcac653156eff53de495165dc7243db2eee477):

  • frontend/src/components/ui/PasswordInput.tsx - Added Caps Lock detection with portal-based notification

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
frontend/src/components/ui/PasswordInput.tsx 50 Duplicate state update - setIsCapsLockOn(false) is called twice consecutively (lines 49 and 50). The second call is redundant and should be removed.
Files Reviewed (1 file)
  • frontend/src/components/ui/PasswordInput.tsx - 1 issue

Previous Review Context

Previous review found 53 inline comments (DeepSource issues) across multiple files. This incremental change only adds a Caps Lock detection feature to the PasswordInput component.


This review performed by Kilo Code review agent

Comment thread app/api/consumable_orders.py Outdated
delete_stmt = delete_stmt.where(ConsumableOrder.applicant_id == current_user.id)
deleted_row = db.exec(delete_stmt.returning(*ConsumableOrder.__table__.columns)).first()
if deleted_row is not None:
return ConsumableOrder.model_validate(dict(deleted_row._mapping))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access to a protected member _mapping of a client class


Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

Comment thread app/api/inventory.py Outdated
.where(Inventory.id == inventory_id)
.returning(*Inventory.__table__.columns)
).first()
item = Inventory.model_validate(dict(deleted_row._mapping)) if deleted_row else None
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access to a protected member _mapping of a client class


Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

return export_inventory_xlsx(items)


def _register_manual_and_dashboard_routes(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`_register_manual_and_dashboard_routes` has a cyclomatic complexity of 19 with "high" risk


A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.

)


def _create_inventory_items_from_order(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`_create_inventory_items_from_order` has a cyclomatic complexity of 16 with "high" risk


A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.



# 校验 stock-in 的权限和订单状态,确保接口契约与错误文案不变。
def _validate_stock_in_order(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`_validate_stock_in_order` has a cyclomatic complexity of 16 with "high" risk


A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.

Comment thread app/api/users.py


@router.post("/login")
def login(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`login` has a cyclomatic complexity of 17 with "high" risk


A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.

Comment thread app/main.py
# Import models to ensure tables are created
# This is needed for SQLModel to register all models
from app.models import User, Inventory, BorrowLog, ReagentOrder, ConsumableOrder, Announcement # noqa: E402, F401
from app.models import ( # noqa: E402, F401
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused RuntimeState imported from app.models


An object has been imported but is not used anywhere in the file.
It should either be used or the import should be removed.

Comment thread app/main.py
# Import models to ensure tables are created
# This is needed for SQLModel to register all models
from app.models import User, Inventory, BorrowLog, ReagentOrder, ConsumableOrder, Announcement # noqa: E402, F401
from app.models import ( # noqa: E402, F401
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused CommonShelf imported from app.models


An object has been imported but is not used anywhere in the file.
It should either be used or the import should be removed.

Comment thread app/main.py
# Import models to ensure tables are created
# This is needed for SQLModel to register all models
from app.models import User, Inventory, BorrowLog, ReagentOrder, ConsumableOrder, Announcement # noqa: E402, F401
from app.models import ( # noqa: E402, F401
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused CommonShelfOperationLog imported from app.models


An object has been imported but is not used anywhere in the file.
It should either be used or the import should be removed.

Comment thread app/main.py
# Import models to ensure tables are created
# This is needed for SQLModel to register all models
from app.models import User, Inventory, BorrowLog, ReagentOrder, ConsumableOrder, Announcement # noqa: E402, F401
from app.models import ( # noqa: E402, F401
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused ChemicalNameMap imported from app.models


An object has been imported but is not used anywhere in the file.
It should either be used or the import should be removed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

develop 分支的大规模改动合并进 main,重点完善库存链路与 CommonShelf / CAS 主数据接入,并同步更新前端存储体系与导入、SSE、导出等相关能力。

Changes:

  • 前端:补齐认证检查与 Suspense Shell、统一本地存储入口、补充导入/公告/bug 按钮等交互细节
  • 后端:新增 CommonShelf / chemical_name_map / 多类操作日志、升级 SSE 会话失效联动、导入改为 preview/confirm 两阶段
  • 运维/部署:Nginx 静态资源强缓存策略、版本化静态资源生成脚本、cache_version 驱动的启动期缓存清理

Reviewed changes

Copilot reviewed 82 out of 154 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
frontend/src/components/CartImportLoadingScreen.tsx 新增导入页鉴权检查时的加载态
frontend/src/components/BugReportButton.tsx bug 按钮隐藏逻辑迁移到统一 storage
frontend/src/components/BaseForm.tsx 表单布局可配置列数、支持 number 类型、label 控制增强
frontend/src/components/AuthDeferredShell.tsx 新增认证未完成时的页面骨架屏
frontend/src/components/AnnouncementButton.tsx 公告已读状态迁移到统一 storage
frontend/src/components/AnnouncementBanner.tsx 公告关闭状态迁移到统一 storage
frontend/src/api/client.ts axios 拦截器增强:SSE client id、统一 session 失效处理、接口扩展
frontend/src/App.tsx 路由层鉴权/启动 bootstrap 重构,增加 Suspense fallback
frontend/scripts/lib-assets.mjs 新增版本化 lib 静态资源同步脚本
frontend/public/robots.txt 新增 robots.txt,禁止索引
frontend/package.json dev/build 前自动生成版本化静态资源
frontend/index.html 添加 robots meta,禁止索引
frontend/.gitignore 忽略版本化 lib 资源输出
docker/nginx/nginx.conf 新增 Nginx 主配置(alpine nginx 包结构)
docker/nginx/default.conf 静态资源强缓存 + brotli/gzip_static + /lib 路由
docker/frontend/Dockerfile 构建镜像改为 alpine + 安装 nginx/brotli 模块
browser-extension/shared/site-config.js 新增扩展站点配置与 URL 归一化工具
browser-extension/popup/popup.js 订单类型识别模块化、与后台通信封装、字段抽取调整
browser-extension/popup/popup.html 注入订单类型识别脚本
browser-extension/popup/order-type-detection.js 新增订单类型识别/字段抽取/规格提取工具
browser-extension/manifest.json 增加 webRequest 权限以跟踪购物车标签页活跃度
browser-extension/content/import-bridge.js localStorage 清理容错增强
browser-extension/background/service-worker.js 后台统一选取购物车 tab、支持动态站点配置、跟踪活跃度
browser-extension/background/cart-tab-selection.js 新增购物车 tab 选择策略模块
app/services/xlsx_export.py 导出结构调整:库存/订单/常用货架导出字段更新(含 purity)
app/services/user_operation_logger.py 新增用户操作日志 snapshot 记录与解析
app/services/sse_manager.py SSE:订阅请求结构化、会话撤销控制流、actor_client_id 传播
app/services/shelf_utils.py 货架工具收敛到位置规范化
app/services/session_service.py session 生命周期/撤销副作用收口,并与 SSE 联动
app/services/search_matchers.py users FTS bindparam 名称去冲突
app/services/pinyin_utils.py 拼音字段截断常量化,清理注释与函数签名
app/services/order_operation_logger.py 新增订单操作日志 snapshot 记录与解析
app/services/order_fts.py FTS bindparam 名称按表名隔离,避免冲突
app/services/inventory_queries.py inventory 查询入口简化(common shelf 已拆表)
app/services/inventory_operation_logger.py 新增库存操作日志 snapshot 记录与解析
app/services/inventory_import_preview_sessions.py 新增两阶段导入的 Redis/内存预览会话存储
app/services/inventory_creation.py 入库创建补充 purity + 内部编码冲突重试/回滚逻辑
app/services/internal_code.py 内部编码序列从“扫描 max”升级为序列表 + 原子保留区间
app/services/image_service.py 静态路径校验与注释收敛
app/services/common_shelf_operation_logger.py 新增常用货架操作日志 snapshot 记录与解析
app/services/common_shelf_creation.py 新增常用货架创建/规格归一化/内部编码冲突处理
app/services/common_name_utils.py 删除旧的 [std] name marker 工具
app/services/chemical_name_map_fts.py 新增 chemical_name_map 的 FTS 过滤与匹配构造
app/services/cache_reset_service.py 新增 cache_version 驱动的启动期 Redis 清理 + session 全撤销
app/services/audit_logger.py AuditEventContext 结构化上下文
app/services/api_utils.py 缓存裁剪逻辑与常量统一
app/models/user_operation_log.py 新增用户操作日志表与 DTO
app/models/runtime_state.py 新增 runtime_state 表用于启动协调数据持久化
app/models/reagent_order_operation_log.py 新增试剂订单操作日志表与 DTO
app/models/reagent_order.py 新增 purity 字段
app/models/inventory_operation_log.py 新增库存操作日志表与 DTO
app/models/inventory.py inventory 新增 purity、移除 is_common、索引调整
app/models/consumable_order_operation_log.py 新增耗材订单操作日志表与 DTO
app/models/common_shelf_operation_log.py 新增常用货架操作日志表与 DTO
app/models/common_shelf.py 新增 common_shelf 表与相关 DTO/请求模型
app/models/chemical_name_map.py 新增 chemical_name_map 表与 DTO/枚举
app/models/init.py 汇总导出新增模型
app/main.py 新增 robots/health 信息、SSE client id 上下文、中间件与新路由注册
app/core/request_utils.py 新增 request-scoped SSE client id contextvar
app/core/redis.py 新增按前缀删除 Redis keys 的工具与返回结构
app/core/constants.py 导入路径、限流常量、SSE event 常量调整
app/core/config.py 新增 cache_version,并默认回退到 app_version
app/api/user_sessions.py session 列表/删除/刷新/更新逻辑与新 session_service 对齐
app/api/events.py SSE 建连增强:订阅请求携带 session 信息 + 周期复检
app/api/deps.py get_current_session 下沉到 core.auth.resolve_current_session
app/api/chemical_name_map.py 新增 chemical_name_map CRUD API
README.md 文档更新:前端本地存储 key 列表等
.env.example 新增 CACHE_VERSION 配置示例
Files not reviewed (1)
  • frontend/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)

app/services/inventory_creation.py:1

  • 该 helper 在捕获 IntegrityError 时直接 db.rollback(),会回滚当前 Session 内所有未提交变更(不仅仅是本次插入的库存行)。这会让调用方很难在“同一个事务里先做其他更新、再创建库存行”的场景下保证原子性/可预期性。建议把插入与 flush() 包裹在 with db.begin_nested(): 的 savepoint 里(类似 common_shelf_creation.py 的处理),这样内部编码冲突重试只回滚本批次插入,不会影响外层事务的其他写入。
"""Shared inventory item creation helpers."""

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +15 to 23
FROM alpine:3.20

COPY docker/nginx/default.conf /etc/nginx/conf.d/default.conf
RUN apk add --no-cache nginx nginx-mod-http-brotli

COPY docker/nginx/nginx.conf /etc/nginx/nginx.conf
COPY docker/nginx/default.conf /etc/nginx/http.d/default.conf
COPY --from=frontend-builder /app/frontend/dist /usr/share/nginx/html

EXPOSE 80
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

该镜像从 nginx:* 基础镜像切到 alpine 后,不再自带 Nginx 的 entrypoint/CMD。当前 Dockerfile 没有 CMD 启动 Nginx,容器会构建成功但运行时不会监听 80 端口。建议显式添加 CMD(例如以 daemon off; 方式前台运行),并确保运行所需目录(如 /run/nginx)在启动时存在。

Copilot uses AI. Check for mistakes.
Comment thread frontend/src/components/BaseForm.tsx Outdated
Comment on lines 305 to 310
<Input
{...controllerField}
id={fieldId}
type={field.inputType || "text"}
type={field.type === "number" ? "number" : field.inputType || "text"}
value={(controllerField.value as string) ?? ""}
onChange={(event) => controllerField.onChange(event.target.value)}
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里虽然把 field.type === \"number\" 映射成了 <input type=\"number\">,但 onChange 仍然把 event.target.value(字符串)写回表单状态,会导致 number 字段在表单值/校验层面变成字符串(例如 schema/transform、后端提交类型、比较逻辑都会受影响)。建议在 number 类型下使用 event.target.valueAsNumber(并处理空字符串时回传 undefined/null),或结合 react-hook-form 的 valueAsNumber 语义确保存储类型为 number。

Copilot uses AI. Check for mistakes.
Comment thread app/services/chemical_name_map_fts.py Outdated
Comment on lines +73 to +74
match_expr = text("chemical_name_map_fts MATCH :match_query").bindparams(
bindparam("match_query", match_query)
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bindparam(\"match_query\", ...) 采用了通用参数名,在一个查询里如果同时拼接多个 FTS 子句(本仓库其他 FTS 已开始用 ${table}_match_query 避免冲突),会触发 SQLAlchemy 的参数名冲突/覆盖,导致查询结果错误或直接抛错。建议把参数名改为表名相关的唯一值(例如 chemical_name_map_fts_match_query),与 order_fts.py / search_matchers.py 的修复保持一致。

Suggested change
match_expr = text("chemical_name_map_fts MATCH :match_query").bindparams(
bindparam("match_query", match_query)
match_expr = text(
"chemical_name_map_fts MATCH :chemical_name_map_fts_match_query"
).bindparams(
bindparam("chemical_name_map_fts_match_query", match_query)

Copilot uses AI. Check for mistakes.
Comment thread frontend/src/api/client.ts Outdated
const isLogoutRequest = requestUrl.includes('/users/logout')
const skipAuthInvalidation = hasSkipAuthInvalidationHeader(error.config?.headers)
const status = error.response?.status
const authErrorCode = String(error.response?.headers?.['x-auth-error-code'] ?? '')
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里读取 error.response.headers 使用了对象下标方式(headers?.['x-auth-error-code']),但在 axios v1 中 response.headers 可能是 AxiosHeaders 实例而不是 plain object。这样会导致 authErrorCode 读不到,进而 AUTH_USER_DISABLED 等分支不生效。建议复用同文件新增的 readHeaderValue(...) 来读取 response headers(同时兼容大小写),避免拦截器逻辑在不同运行环境下不一致。

Suggested change
const authErrorCode = String(error.response?.headers?.['x-auth-error-code'] ?? '')
const authErrorCode = String(
readHeaderValue(error.response?.headers, 'x-auth-error-code')
?? readHeaderValue(error.response?.headers, 'X-Auth-Error-Code')
?? ''
)

Copilot uses AI. Check for mistakes.
Comment thread app/services/internal_code.py Outdated
Comment on lines +124 to +143
reserved_row = session.exec(
text(
"""
UPDATE internal_code_sequences
SET current_seq = current_seq + :quantity,
updated_at = CURRENT_TIMESTAMP
WHERE prefix = :prefix
AND current_seq + :quantity <= :max_sequence
RETURNING current_seq
"""
),
{
"prefix": prefix,
"quantity": quantity,
"max_sequence": INTERNAL_CODE_MAX_SEQUENCE,
},
).first()
if reserved_row is None:
raise ValueError("internal code sequence limit reached")
return _extract_scalar(reserved_row)
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UPDATE ... RETURNING 依赖数据库对 RETURNING 的支持(SQLite 需要 >= 3.35)。如果生产环境/容器内 SQLite 版本偏旧,会在生成内部编码时直接报错并导致入库/常用货架创建不可用。建议在运行时启动自检(或文档/镜像中锁定 SQLite 版本),并提供不支持 RETURNING 的 fallback(例如先 UPDATE,再 SELECT current_seq)以降低环境差异带来的上线风险。

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5da8260b56

ℹ️ 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".

return len(expired_token_hashes)
if session.token_hash != old_token_hash:
delete_cached_session(old_token_hash)
sse_manager.notify_session_revoked(token_hash=old_token_hash, reason="session_refreshed")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid revoking active SSE clients on self session refresh

When /users/me/sessions/refresh rotates the token hash, this code publishes a session-revoked signal for the old hash immediately. Any other tab that still has an SSE stream on that hash receives auth.invalid and the frontend invalidation path force-logs the user out, so a normal “refresh session” action in one tab can unexpectedly sign out active tabs. This should not emit a hard revocation for same-user refresh without a reconnect/grace flow.

Useful? React with 👍 / 👎.

Comment thread app/main.py
Comment on lines +190 to +192
if path.startswith("/api"):
return True
return path in UNAUTH_REDIRECT_EXEMPT_PATHS
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Exempt docs/openapi routes from unauthenticated redirect

The redirect bypass only allows /api, /login, /robots.txt, and /health. In development-like environments where FastAPI docs are enabled (/docs, /redoc, /openapi.json), requests are redirected to /login before reaching the docs handlers, effectively disabling API docs access. Add docs/openapi paths to the bypass list (or gate this middleware by environment) to avoid breaking local/debug workflows.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

18 issues found across 154 files

Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/services/session_service.py">

<violation number="1" location="app/services/session_service.py:356">
P2: `_create_user_session` is dead code — it is defined but never called. The login flow now uses `stage_create_or_refresh_user_session` with explicit post-commit side effects. Remove this function to avoid maintenance burden on an unused code path.</violation>
</file>

<file name="app/models/common_shelf_operation_log.py">

<violation number="1" location="app/models/common_shelf_operation_log.py:54">
P1: Using `ondelete="CASCADE"` on an audit log table means deleting a user will silently destroy their entire operation log history. Other operation log models in this codebase (`reagent_order_operation_log`, `consumable_order_operation_log`) use `ondelete="SET NULL"` with `Optional[int]` to preserve audit trails. This field should follow the same pattern.</violation>
</file>

<file name="README.md">

<violation number="1" location="README.md:23">
P2: The `开发约定` section was removed but its TOC entry was not. This leaves a broken anchor link in the Table of Contents. Remove the orphaned `- [开发约定](#开发约定)` entry.</violation>
</file>

<file name="app/models/common_shelf.py">

<violation number="1" location="app/models/common_shelf.py:154">
P2: `CommonShelfManualCreate` is missing `model_config = ConfigDict(extra="forbid")`, unlike every other request model in this file. This allows clients to send unexpected extra fields without error, which is inconsistent and can mask typos or malicious payloads.</violation>
</file>

<file name="app/api/events.py">

<violation number="1" location="app/api/events.py:106">
P1: Blocking synchronous DB call inside an async generator will stall the event loop. `is_token_session_active` can fall through the Redis cache and execute `with Session(engine) as db`, which is synchronous I/O. Wrap this in `await asyncio.to_thread(...)` (or `loop.run_in_executor`) so the revalidation doesn't block all concurrent SSE streams and other async handlers.</violation>
</file>

<file name="app/core/auth.py">

<violation number="1" location="app/core/auth.py:119">
P2: `is_token_session_active` does not sync session back to Redis after a successful DB fallback. Unlike `resolve_current_session` which calls `sync_session_cache` on cache miss, this path returns `True` without repopulating the cache. Since SSE connections revalidate every 15 seconds, a cold cache causes a repeated DB JOIN query for the entire connection lifetime.</violation>
</file>

<file name="app/services/inventory_operation_logger.py">

<violation number="1" location="app/services/inventory_operation_logger.py:81">
P2: `status` is never included in the snapshot even though `SNAPSHOT_KEY_MAP` defines `"st" → "status"` and the `Inventory` model has a `status: InventoryStatus` field. Audit log entries will not capture whether the item was `in_stock`, `borrowed`, `consumed`, etc. at the time of the operation.</violation>
</file>

<file name="app/api/user_logs.py">

<violation number="1" location="app/api/user_logs.py:151">
P1: `int(None)` raises `TypeError`, which is not caught. If the payload is valid JSON but lacks `uid` or `exp`, `payload.get(...)` returns `None` and this will bubble up as a 500 instead of being treated as an invalid token. Add `TypeError` to the except clause.</violation>
</file>

<file name="app/services/common_shelf_queries.py">

<violation number="1" location="app/services/common_shelf_queries.py:410">
P2: The inline field list in the 'all' fallback duplicates `CHEMICAL_NAME_MAP_SQL_FIELD_MAP["name"] + CHEMICAL_NAME_MAP_SQL_FIELD_MAP["alias"]`. If a column is added to the map but not here, "all"-field searches silently miss it. Derive the list from the map constant instead.</violation>
</file>

<file name="app/database.py">

<violation number="1" location="app/database.py:173">
P3: `SQLITE_INVENTORY_OPERATION_LOG_INDEX_UPGRADES` is defined but never used anywhere. All five indexes it declares already exist in `SQLITE_PERFORMANCE_FILTER_SORT_INDEX_UPGRADES`, so this constant is dead code that could mislead future maintainers into thinking it serves a separate purpose.</violation>
</file>

<file name="app/models/inventory_operation_log.py">

<violation number="1" location="app/models/inventory_operation_log.py:72">
P1: Audit log uses `CASCADE` on `operator_id` — deleting a user will destroy their inventory operation log history. Every other operation log model in the codebase uses `ondelete="SET NULL"` with `Optional[int]` to preserve audit records. Apply the same pattern here.</violation>
</file>

<file name="app/main.py">

<violation number="1" location="app/main.py:197">
P2: Passing `background_tasks=None` causes `resolve_current_session` to run the activity-refresh task synchronously (DB reads + writes) inside this async middleware, blocking the event loop. Since only auth verification is needed here—not session activity tracking—consider using a lighter-weight auth check that skips the activity refresh, or explicitly suppress the refresh.</violation>
</file>

<file name="app/api/user_sessions.py">

<violation number="1" location="app/api/user_sessions.py:113">
P1: Returning 401 with `SESSION_EXPIRED` auth-error headers when the *target* session (not the caller's auth session) is expired will mislead the frontend into treating this as an authentication failure. Since the caller is authenticated via `get_current_session`, this should be a 404 or 410 to indicate the target session is no longer valid.</violation>
</file>

<file name="docker/nginx/nginx.conf">

<violation number="1" location="docker/nginx/nginx.conf:26">
P3: Add `server_tokens off;` in the `http` block to suppress the nginx version from response headers and error pages. This is a standard security hardening measure for production deployments.</violation>
</file>

<file name="frontend/scripts/lib-assets.mjs">

<violation number="1" location="frontend/scripts/lib-assets.mjs:88">
P2: The `catch` block swallows the inner "not a file" error thrown when `isFile()` is false, replacing it with a misleading "missing source file" message. Re-throw known errors or check the error type before replacing.</violation>
</file>

<file name="app/services/common_shelf_creation.py">

<violation number="1" location="app/services/common_shelf_creation.py:207">
P2: Missing positive-value check on `order.initial_quantity` — a negative value will produce an invalid specification (e.g. "-5 mL") and be persisted to `CommonShelf`. The manual-create path validates `value > 0` via `parse_specification`, but this order-based path does not.</violation>
</file>

<file name="app/services/inventory_import_preview_sessions.py">

<violation number="1" location="app/services/inventory_import_preview_sessions.py:199">
P1: The `token` is embedded into a Redis `SCAN` glob pattern without escaping metacharacters (`*`, `?`, `[`). A token containing `*` would match and delete all preview session keys across all users. Escape the token or scope the key by `user_id` (as `consume_*` does) to build an exact key for `DELETE` instead of scanning.</violation>
</file>

<file name="app/services/internal_code.py">

<violation number="1" location="app/services/internal_code.py:111">
P1: The parameter dict is passed as a positional argument to `session.exec()`, but SQLModel's `Session.exec()` declares `params` as keyword-only. This would raise a `TypeError` at runtime. Use `params={...}` instead. The same issue exists in `_reserve_sequence_range` below.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread app/models/common_shelf_operation_log.py Outdated
Comment thread app/api/events.py

now_utc = get_utc_now()
if now_utc >= next_revalidate_at:
if not is_token_session_active(current_session.token_hash, client_ip=client_ip):
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Blocking synchronous DB call inside an async generator will stall the event loop. is_token_session_active can fall through the Redis cache and execute with Session(engine) as db, which is synchronous I/O. Wrap this in await asyncio.to_thread(...) (or loop.run_in_executor) so the revalidation doesn't block all concurrent SSE streams and other async handlers.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/events.py, line 106:

<comment>Blocking synchronous DB call inside an async generator will stall the event loop. `is_token_session_active` can fall through the Redis cache and execute `with Session(engine) as db`, which is synchronous I/O. Wrap this in `await asyncio.to_thread(...)` (or `loop.run_in_executor`) so the revalidation doesn't block all concurrent SSE streams and other async handlers.</comment>

<file context>
@@ -84,10 +94,24 @@ async def event_generator():
+
+            now_utc = get_utc_now()
+            if now_utc >= next_revalidate_at:
+                if not is_token_session_active(current_session.token_hash, client_ip=client_ip):
+                    sse_manager.notify_session_revoked(
+                        token_hash=current_session.token_hash,
</file context>
Fix with Cubic

Comment thread app/api/user_logs.py
expires_at = int(parts[1])
return (user_id, expires_at)
except (ValueError, IndexError):
except (ValueError, IndexError, json.JSONDecodeError, UnicodeDecodeError):
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: int(None) raises TypeError, which is not caught. If the payload is valid JSON but lacks uid or exp, payload.get(...) returns None and this will bubble up as a 500 instead of being treated as an invalid token. Add TypeError to the except clause.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/user_logs.py, line 151:

<comment>`int(None)` raises `TypeError`, which is not caught. If the payload is valid JSON but lacks `uid` or `exp`, `payload.get(...)` returns `None` and this will bubble up as a 500 instead of being treated as an invalid token. Add `TypeError` to the except clause.</comment>

<file context>
@@ -81,27 +119,40 @@ def _record_logs_token_request(admin_user_id: int) -> None:
-        expires_at = int(parts[1])
         return (user_id, expires_at)
-    except (ValueError, IndexError):
+    except (ValueError, IndexError, json.JSONDecodeError, UnicodeDecodeError):
         return None
 
</file context>
Suggested change
except (ValueError, IndexError, json.JSONDecodeError, UnicodeDecodeError):
except (ValueError, TypeError, IndexError, json.JSONDecodeError, UnicodeDecodeError):
Fix with Cubic


id: Optional[int] = Field(default=None, primary_key=True)
inventory_id: int = Field(index=False)
operator_id: int = Field(
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Audit log uses CASCADE on operator_id — deleting a user will destroy their inventory operation log history. Every other operation log model in the codebase uses ondelete="SET NULL" with Optional[int] to preserve audit records. Apply the same pattern here.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/models/inventory_operation_log.py, line 72:

<comment>Audit log uses `CASCADE` on `operator_id` — deleting a user will destroy their inventory operation log history. Every other operation log model in the codebase uses `ondelete="SET NULL"` with `Optional[int]` to preserve audit records. Apply the same pattern here.</comment>

<file context>
@@ -0,0 +1,106 @@
+
+    id: Optional[int] = Field(default=None, primary_key=True)
+    inventory_id: int = Field(index=False)
+    operator_id: int = Field(
+        foreign_key="users.id",
+        ondelete="CASCADE",
</file context>
Fix with Cubic

Comment thread app/api/user_sessions.py
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="No active session"
status_code=status.HTTP_401_UNAUTHORIZED,
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Returning 401 with SESSION_EXPIRED auth-error headers when the target session (not the caller's auth session) is expired will mislead the frontend into treating this as an authentication failure. Since the caller is authenticated via get_current_session, this should be a 404 or 410 to indicate the target session is no longer valid.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/user_sessions.py, line 113:

<comment>Returning 401 with `SESSION_EXPIRED` auth-error headers when the *target* session (not the caller's auth session) is expired will mislead the frontend into treating this as an authentication failure. Since the caller is authenticated via `get_current_session`, this should be a 404 or 410 to indicate the target session is no longer valid.</comment>

<file context>
@@ -93,76 +86,86 @@ def delete_all_sessions(
         raise HTTPException(
-            status_code=status.HTTP_404_NOT_FOUND,
-            detail="No active session"
+            status_code=status.HTTP_401_UNAUTHORIZED,
+            detail="Session has been revoked, please login again",
+            headers={
</file context>
Fix with Cubic

Comment thread app/main.py

def _is_authenticated_request(request: Request) -> bool:
with Session(engine) as db:
resolve_current_session(request=request, background_tasks=None, db=db)
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Passing background_tasks=None causes resolve_current_session to run the activity-refresh task synchronously (DB reads + writes) inside this async middleware, blocking the event loop. Since only auth verification is needed here—not session activity tracking—consider using a lighter-weight auth check that skips the activity refresh, or explicitly suppress the refresh.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/main.py, line 197:

<comment>Passing `background_tasks=None` causes `resolve_current_session` to run the activity-refresh task synchronously (DB reads + writes) inside this async middleware, blocking the event loop. Since only auth verification is needed here—not session activity tracking—consider using a lighter-weight auth check that skips the activity refresh, or explicitly suppress the refresh.</comment>

<file context>
@@ -147,6 +176,28 @@ def _should_skip_https_redirect(path: str) -> bool:
+
+def _is_authenticated_request(request: Request) -> bool:
+    with Session(engine) as db:
+        resolve_current_session(request=request, background_tasks=None, db=db)
+    return True
+
</file context>
Fix with Cubic

if (!sourceStat.isFile()) {
throw new Error(`${definition.sourceName} 不是文件`)
}
} catch {
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The catch block swallows the inner "not a file" error thrown when isFile() is false, replacing it with a misleading "missing source file" message. Re-throw known errors or check the error type before replacing.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/scripts/lib-assets.mjs, line 88:

<comment>The `catch` block swallows the inner "not a file" error thrown when `isFile()` is false, replacing it with a misleading "missing source file" message. Re-throw known errors or check the error type before replacing.</comment>

<file context>
@@ -0,0 +1,148 @@
+      if (!sourceStat.isFile()) {
+        throw new Error(`${definition.sourceName} 不是文件`)
+      }
+    } catch {
+      throw new Error(`缺少源资源文件: ${sourcePath}`)
+    }
</file context>
Fix with Cubic

"""Create common shelf rows from a common-public reagent order."""
if order.quantity is None or order.quantity <= 0:
raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid order quantity")
if order.initial_quantity is None or order.unit is None:
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Missing positive-value check on order.initial_quantity — a negative value will produce an invalid specification (e.g. "-5 mL") and be persisted to CommonShelf. The manual-create path validates value > 0 via parse_specification, but this order-based path does not.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/services/common_shelf_creation.py, line 207:

<comment>Missing positive-value check on `order.initial_quantity` — a negative value will produce an invalid specification (e.g. "-5 mL") and be persisted to `CommonShelf`. The manual-create path validates `value > 0` via `parse_specification`, but this order-based path does not.</comment>

<file context>
@@ -0,0 +1,300 @@
+    """Create common shelf rows from a common-public reagent order."""
+    if order.quantity is None or order.quantity <= 0:
+        raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid order quantity")
+    if order.initial_quantity is None or order.unit is None:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
</file context>
Fix with Cubic

Comment thread app/database.py
+ SQLITE_PERFORMANCE_FILTER_SORT_INDEX_UPGRADES
)

SQLITE_INVENTORY_OPERATION_LOG_INDEX_UPGRADES: tuple[str, ...] = (
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: SQLITE_INVENTORY_OPERATION_LOG_INDEX_UPGRADES is defined but never used anywhere. All five indexes it declares already exist in SQLITE_PERFORMANCE_FILTER_SORT_INDEX_UPGRADES, so this constant is dead code that could mislead future maintainers into thinking it serves a separate purpose.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/database.py, line 173:

<comment>`SQLITE_INVENTORY_OPERATION_LOG_INDEX_UPGRADES` is defined but never used anywhere. All five indexes it declares already exist in `SQLITE_PERFORMANCE_FILTER_SORT_INDEX_UPGRADES`, so this constant is dead code that could mislead future maintainers into thinking it serves a separate purpose.</comment>

<file context>
@@ -118,6 +170,14 @@ def get_db() -> Generator[Session, None, None]:
     + SQLITE_PERFORMANCE_FILTER_SORT_INDEX_UPGRADES
 )
 
+SQLITE_INVENTORY_OPERATION_LOG_INDEX_UPGRADES: tuple[str, ...] = (
+    "CREATE INDEX IF NOT EXISTS ix_inventory_operation_log_operator_created_at ON inventory_operation_log (operator_id, created_at DESC)",
+    "CREATE INDEX IF NOT EXISTS ix_inventory_operation_log_action_created_at ON inventory_operation_log (action, created_at DESC)",
</file context>
Fix with Cubic

Comment thread docker/nginx/nginx.conf
sendfile on;
tcp_nopush on;
keepalive_timeout 65;
types_hash_max_size 4096;
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Add server_tokens off; in the http block to suppress the nginx version from response headers and error pages. This is a standard security hardening measure for production deployments.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docker/nginx/nginx.conf, line 26:

<comment>Add `server_tokens off;` in the `http` block to suppress the nginx version from response headers and error pages. This is a standard security hardening measure for production deployments.</comment>

<file context>
@@ -0,0 +1,29 @@
+    sendfile on;
+    tcp_nopush on;
+    keepalive_timeout 65;
+    types_hash_max_size 4096;
+
+    include /etc/nginx/http.d/*.conf;
</file context>
Fix with Cubic

@hzb666
Copy link
Copy Markdown
Owner Author

hzb666 commented Mar 31, 2026

@copilot 审查这个pr,确保每一个修改的代码都要看

…operation_log

Prevents silent deletion of audit log history when a user is deleted.
Aligns with the pattern used by reagent_order_operation_log and
consumable_order_operation_log.
…mber input onChange, and SQLite RETURNING fallback

Agent-Logs-Url: https://github.com/hzb666/LabStorageManager/sessions/70d909b4-1246-4e9d-a115-72f5f98fda88

Co-authored-by: hzb666 <29155232+hzb666@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI commented Mar 31, 2026

@copilot 审查这个pr,确保每一个修改的代码都要看

已完成全面审查并修复了所有代码审查意见(commit 376faac):

  1. BaseForm.tsxrenderInputFieldtype="number"onChange 改用 event.target.valueAsNumber,并将 NaN 转为 undefined;同时移除 value 上不必要的 as string 强转。
  2. chemical_name_map_fts.pybindparam 参数名从通用的 match_query 改为 chemical_name_map_fts_match_query,与 order_fts.py / search_matchers.py 的命名规范保持一致,避免多 FTS 子查询拼接时的参数名冲突。
  3. client.ts:读取响应头 x-auth-error-code 改用文件内已有的 readHeaderValue() 工具函数(同时覆盖小写/大写),兼容 AxiosHeaders 实例与普通对象。
  4. internal_code.py:新增模块级 _SQLITE_SUPPORTS_RETURNING 标志(在导入时检测 sqlite3.sqlite_version),并在 _reserve_sequence_range 中提供兼容 SQLite < 3.35 的 UPDATE + SELECT 回退路径。
  5. Dockerfile:确认 CMD ["nginx", "-g", "daemon off;"] 已存在,无需修改。

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/src/components/BaseForm.tsx">

<violation number="1">
P2: Number parsing relies on `valueAsNumber`, but `Input`'s synthetic change events only carry `target.value`, so stepper-triggered updates for `type: 'number'` fields can be lost.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

@hzb666
Copy link
Copy Markdown
Owner Author

hzb666 commented Mar 31, 2026

@copilot _SQLITE_SUPPORTS_RETURNING是不是还有其他地方也用了returning,你也要做好回退

…es via shared db_compat helpers

Agent-Logs-Url: https://github.com/hzb666/LabStorageManager/sessions/d6b5a2ea-0c3f-4935-9a03-43cde20c8f49

Co-authored-by: hzb666 <29155232+hzb666@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI commented Mar 31, 2026

@copilot _SQLITE_SUPPORTS_RETURNING是不是还有其他地方也用了returning,你也要做好回退

已全面排查并修复(commit ebe7b77)。

internal_code.pyUPDATE … RETURNING 外,还有 5 处 DELETE … RETURNING

文件 函数
app/services/common_shelf_queries.py remove_earliest_item_in_location
app/services/common_shelf_queries.py delete_group_items_returning
app/api/inventory.py delete_inventory
app/api/reagent_orders_workflow.py _delete_reagent_order_with_permission
app/api/consumable_orders.py delete order

新增 app/core/db_compat.py,统一定义 SQLITE_SUPPORTS_RETURNING 标志(模块加载时检测 sqlite3.sqlite_version)以及两个辅助函数 exec_delete_returning_first / exec_delete_returning_all。所有 DELETE … RETURNING 调用均改用这两个函数,在 SQLite < 3.35 时自动回退为 SELECT-then-DELETE。internal_code.py_SQLITE_SUPPORTS_RETURNING 也改为从 db_compat 导入,消除了重复的版本检测逻辑。

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/core/db_compat.py">

<violation number="1" location="app/core/db_compat.py:38">
P2: Fallback logic breaks DELETE statements that have no WHERE clause on SQLite < 3.35, because `.where(None)` produces `WHERE NULL` and causes an early return before deletion.</violation>

<violation number="2" location="app/core/db_compat.py:58">
P2: `exec_delete_returning_all` has the same `.where(None)` fallback bug, causing unconditional deletes to return `[]` and skip the delete.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread app/core/db_compat.py
rows = db.exec(delete_stmt.returning(*model_cls.__table__.columns)).all()
return [model_cls.model_validate(dict(row._mapping)) for row in rows]
# Fallback: fetch all matching rows first, then delete them.
existing = db.exec(select(model_cls).where(delete_stmt.whereclause)).all()
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: exec_delete_returning_all has the same .where(None) fallback bug, causing unconditional deletes to return [] and skip the delete.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/core/db_compat.py, line 58:

<comment>`exec_delete_returning_all` has the same `.where(None)` fallback bug, causing unconditional deletes to return `[]` and skip the delete.</comment>

<file context>
@@ -0,0 +1,62 @@
+        rows = db.exec(delete_stmt.returning(*model_cls.__table__.columns)).all()
+        return [model_cls.model_validate(dict(row._mapping)) for row in rows]
+    # Fallback: fetch all matching rows first, then delete them.
+    existing = db.exec(select(model_cls).where(delete_stmt.whereclause)).all()
+    if not existing:
+        return []
</file context>
Suggested change
existing = db.exec(select(model_cls).where(delete_stmt.whereclause)).all()
select_stmt = select(model_cls)
if delete_stmt.whereclause is not None:
select_stmt = select_stmt.where(delete_stmt.whereclause)
existing = db.exec(select_stmt).all()
Fix with Cubic

Comment thread app/core/db_compat.py
return None
return model_cls.model_validate(dict(row._mapping))
# Fallback: fetch the row first, then delete it.
existing = db.exec(select(model_cls).where(delete_stmt.whereclause)).first()
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Fallback logic breaks DELETE statements that have no WHERE clause on SQLite < 3.35, because .where(None) produces WHERE NULL and causes an early return before deletion.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/core/db_compat.py, line 38:

<comment>Fallback logic breaks DELETE statements that have no WHERE clause on SQLite < 3.35, because `.where(None)` produces `WHERE NULL` and causes an early return before deletion.</comment>

<file context>
@@ -0,0 +1,62 @@
+            return None
+        return model_cls.model_validate(dict(row._mapping))
+    # Fallback: fetch the row first, then delete it.
+    existing = db.exec(select(model_cls).where(delete_stmt.whereclause)).first()
+    if existing is None:
+        return None
</file context>
Suggested change
existing = db.exec(select(model_cls).where(delete_stmt.whereclause)).first()
select_stmt = select(model_cls)
if delete_stmt.whereclause is not None:
select_stmt = select_stmt.where(delete_stmt.whereclause)
existing = db.exec(select_stmt).first()
Fix with Cubic

Comment thread app/core/db_compat.py
row = db.exec(delete_stmt.returning(*model_cls.__table__.columns)).first()
if row is None:
return None
return model_cls.model_validate(dict(row._mapping))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access to a protected member _mapping of a client class


Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

Comment thread app/core/db_compat.py
"""
if SQLITE_SUPPORTS_RETURNING:
rows = db.exec(delete_stmt.returning(*model_cls.__table__.columns)).all()
return [model_cls.model_validate(dict(row._mapping)) for row in rows]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access to a protected member _mapping of a client class


Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

onBlur={(event) => {
setIsFocused(false)
setIsCapsLockOn(false)
setIsCapsLockOn(false)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Duplicate state update - setIsCapsLockOn(false) is called twice consecutively (lines 49 and 50). The second call is redundant and should be removed.

@hzb666
Copy link
Copy Markdown
Owner Author

hzb666 commented Apr 1, 2026

@copilot 总结一下现在评论区里的内容,不要改代码

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/src/components/ui/PasswordInput.tsx">

<violation number="1">
P3: Duplicate `setIsCapsLockOn(false)` call — remove the extra line.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Copy link
Copy Markdown

Copilot AI commented Apr 1, 2026

@copilot 总结一下现在评论区里的内容,不要改代码

当前评论区结论如下:

  1. 审查机器人最初提了 5 个点(Docker CMD、BaseForm number、FTS bindparam 命名、AxiosHeaders 读取、SQLite RETURNING 兼容)。
  2. 这些点已逐项处理;其中 Dockerfile 已确认原本就有 CMD ["nginx", "-g", "daemon off;"]
  3. 你后续追问“是否还有其他 RETURNING 需要回退”后,已继续全量排查并补齐:除 internal_code.py 外,另外 5 处 DELETE ... RETURNING 也都加了兼容回退,并抽成 app/core/db_compat.py 统一处理。
  4. 目前最新评论是“总结评论区内容,不改代码”,已按此执行,无新增代码变更。

本次回复对应的是纯后端/评审总结,没有 UI 改动可截图。

@hzb666 hzb666 merged commit 4e6beee into main Apr 1, 2026
12 of 16 checks passed
@hzb666 hzb666 deleted the develop branch April 1, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants