Add math lib 添加数学库 - #122
Conversation
Reason: script execution via -e/-c flag Reply |
代码审查摘要 — PR #122 Add math lib 添加数学库操作: opened · 范围: 25 个文件（21 Java 新增，23 新文件 / 2358 行 diff）/ 目标分支 脚手架部分（
🔴 关键1.
右侧以数字结尾/开头即触发（ 2. 负数字面量（
对象形式（ 3.
（顶层纯常量在 4. CI（ jarJar(api("dev.anvilcraft.lib:anvillib-math-neoforge-1.21.1:latest.release"))缺条目 → { "module": "math", "needs": ["util"] }
|
| 被测目标 | 推荐场景 | 优先级 |
|---|---|---|
FlatExpressionParser ↔ FlatExpressionWriter |
属性测试：随机表达式树 parse→write→parse 值与结构一致（本次已证实可稳定复现多类失败） |
🔴 |
number() / parseNumber() |
边界 1e7、1e-3、1e-4、1e8、负数、0、Math.rint 边界 |
🔴 |
visitBinary |
常量×常量、常量×常量（负）、常量×符号、pow 负底数/负指数 |
🔴 |
IFunction.CODEC / IExpression.CODEC |
JSON→对象→JSON 与 NBT→对象→NBT 往返（含数据包函数引用与内联定义） | 🔴 |
FlatExpressionParser.CACHE |
连续多次 reload 后旧注册表是否仍被强引用（弱引用/清理断言） |
🟡 |
CustomFunction.apply |
实参数 ≠ 声明参数（多/少/0 个）、自引用与 A↔B 环 | 🟡 |
LibBuiltInFunctions.callChecked |
每个内建函数的 arity 上下界与错误消息 | 🟢 |
📋 标题
gh 当前未认证（token invalid），未自动修改。建议补充到：
Add math lib: flat expression parser & function registry 新增数学库：表达式文本解析与函数注册表
结论：REQUEST_CHANGES — 脚手架质量高、设计清晰，但表达式回写与解析器之间存在多处不对称：常量并置（2*3→23）、负数字面量、Double.toString 指数形式都会让「写出去的数据读不回来或读回来变了值」，而这正好是存档/网络同步走的编码路径；另外新模块未接入 CI 矩阵。建议优先修 #1–#4，并把 #1–#3 固化为属性测试。
由 Hermes Agent 审查
🌿 Roseau API Breaking Change Report
|
## 代码审查摘要 — PR #122（converted_to_draft：仅报严重问题，不阻塞性建议从略）
**操作:** converted_to_draft
**范围:** 25 个文件 / 2358 行 diff（23 新增文件，全部 Java + 构建脚本 + 资源；0 删除）
**范围:** 新增 Gradle 子模块 `module.math`（anvillib-math）：表达式树 / flat 文本 ↔ 表达式 codec / 函数与函数类型注册表
---
### 🔴 关键
**1. `.github/modules.json` 未更新 → 新模块不构建、不发布，且会打断 main 的发布流程**
本 PR 只改了 `settings.gradle`（`include 'module.math'` + 项目名映射）和 `module.main/build.gradle`（`jarJar(api("dev.anvilcraft.lib:anvillib-math-neoforge-1.21.1:latest.release"))`），但仓库的构建/发布矩阵完全由 `.github/modules.json` 经 `.github/workflows/generate-matrix.js` 生成（`ci.yml` / `pull_request.yml` / `release.yml` 都只跑 `level_0..2` 的 matrix）：
- `math` 不在 modules.json 里 → CI/Release 都不会构建 `module.math`，本模块的编译错误在 CI 里查不出来，也不会发布到 Maven Central；
- `release.yml` 走 `NOT_DEV=true` 分支，`module.main` 会去解析 `anvillib-math-neoforge-1.21.1:latest.release` —— 该坐标永远不会存在 → main 的发布流程直接失败。
- 依赖关系上 `module.math/build.gradle` 已 `jarJar(implementation project(":anvillib-util-neoforge-1.21.1"))`，`IFunction` 也 import 了 `dev.anvilcraft.lib.v2.util.ISerializer`。
建议补：`{ "module": "math", "needs": ["util"] }`（同时 `roseau_check` 的 `module_names` 也来自该文件，会一并漏掉 math）。
**2. `FlatExpressionWriter` 回写文本与解析器语法不自洽：常量×常量并置乘法静默改值（已实测）**
`visitBinary` 在 `isLiteralNumber(left)` 时把 `MULTIPLY` 写成并置，但右操作数**本身也是数字字面量**时会被左数字吞并：
| 表达式树 | 回写文本 | 重新解析结果 | 值变化 |
|---|---|---|---|
| `multiply(2, 3)`（即文本 `2*3`） | `23` | `23` | **6 → 23** |
| `multiply(2.5, 100)` | `2.5100` | `2.51` | **250 → 2.51** |
| `multiply(2, 0.5)` | `20.5` | `20.5` | **1 → 20.5** |
| `multiply(2.5, 2.5)` | `2.52.5` | 解析失败 | 表达式无法加载 |
| `multiply(2, -1)` | `2-1` | `subtract(2, 1)` | **-2 → 1** |
| `multiply(-2, 3)` | `-23` | `-(23)` | **-6 → -23** |
| `multiply(-2, -3)` | `-2-3` | `-2-3` | **6 → -5** |
注意 `multiply(2,3)` 正是解析 `"2*3"` 得到的树——**最基础的输入就无法往返**。触发面不止数据包存档：`IExpression.STREAM_CODEC = ByteBufCodecs.fromCodecWithRegistries(IExpression.CODEC)` 走的也是这条 `encode` 路径（`FunctionExpression.FLAT_OR_OBJECT_CODEC` 的 `Codec.xor` `/ xmap(..., Either::left)` 永远先写 flat 文本），所以网络同步会把被改写的文本发到客户端 → 客户端解析出不同数值，或解析失败断连。
**3. 负数字面量永不补括号（`parenthesize` 对常量短路）**
`Operator.of()` 只识别 5 个内建运算符，`ConstantFunction` 命中 `default -> null` → `parenthesize()` 在 `child == null` 处直接返回 `false`，于是负数字面量在任何位置都不会加括号：
- `pow(-2, 2)` → `-2^2`，重新解析为 `subtract(0, pow(2,2))` → **4 → -4**
- 与问题 2 叠加即产生 `2-1` / `-23` 这类并置乘法错误。
建议：`parenthesize()` 对常量按「负值在幂底数 / 并置乘法任一侧必须括号」处理，或对负数字面量统一加括号。
**4. `number()` 使用 `Double.toString` → 指数形式写出后读不回来**
`number()` 只在 `value == Math.rint(value) && |value| < 1e7` 时走 `Long.toString`，其余落回 `Double.toString`，会输出 `1.0E7` / `1.0E-4` 这类指数形式；而 `parseNumber()` 只扫描 `digits[.digits]`，`E` 被当成标识符 → 报 `expected '(' after function 'E7'`：
- `x*20000000` → `x*2.0E7` → 回读失败
- `x*0.0001` → `x*1.0E-4` → 回读失败
（顶层常量因为有 `Codec.either(DOUBLE, flat)` 第一支兜住不受影响，只有嵌套子项会踩到。）这是**写得出、读不回**：存档重载 / 数据包 reload / 网络包解码直接失败。
### ⚠️ 警告
**5. 数据包函数 id 路径含 `/` 无法往返**：`functionName()` 用 `stripDefaultNamespace(key.location())` / `id.toString()` 原样输出，但 `isIdentifierPart()` 不含 `/`。`data/<ns>/anvillib/function/utils/triple.json`（子目录是数据包常规写法）→ 写出 `mymod:utils/triple(x)` / `utils/triple(x)` → 回读报 `expected '(' after function 'mymod:utils'`。建议 writer 对含非法字符的名字返回 `Optional.empty()`（自然回退到对象形式），或让标识符扫描器接受 `/`。
**6. 内建函数遮蔽其他命名空间的同名函数**：`parseIdentifier` 用 `withDefaultNamespace(lower).getPath()` 查内建，与命名空间无关 → 实测 `othermod:max(x,1)` 被解析成内建 `MAX`；而 writer 又原样写 `othermod:max(x,1)`，回读变成内建 → 树与值都变。建议只在「无命名空间 或 namespace == MAIN_ID」时查内建。
**7. 注册命名空间混用，且 javadoc 示例与实际注册名不一致**：同一 `FUNCTION_TYPE` 注册表里 `LibBuiltInFunctions.TYPE_DF` 用 `MAIN_ID`（`anvillib:builtin`）、`LibFunctionTypes.DF` 用 `MOD_ID`（`anvillib_math:input/named/constant/custom`）；而 `CustomFunction` / `ConstantFunction` 的示例写的是 `"type": "anvillib:custom"`、`"anvillib:constant"` → 按文档照抄的 JSON 会因为找不到类型而加载失败。仓库里两种先例都有（`LibItemSubPredicates` 用 MAIN_ID、`AnvilLibSyncEntries` 用 MOD_ID），但同一个 PR 内至少要对齐，并保证文档示例可直接加载。
**8. 静态解析缓存无上限且永不清理**：`CACHE: Map<HolderGetter<IFunction>, Map<String, IExpression>>` 每次数据包 reload / 换存档都会换一个新的 registry 实例作为 key，旧实例与其中缓存的全部表达式树被强引用永久保留；`source` 侧也无上限（网络传入任意字符串都会入缓存）。建议弱引用 key 或在 reload 事件里清空。
### 💡 建议（不阻塞）
- **数据包函数缺 arity 校验**：只有内建函数走 `callChecked`，注册表函数（`CustomFunction`）没有参数个数信息，`apply()` 用 `min(parameters.size(), arguments.size())` 循环 → 多传的实参被静默丢弃、少传的 `$(name)` 取 0。建议在解析期按 `parameters.size()` 校验。
- **`CustomFunction` 无环/深度检测**：body 引用自身或互相引用时求值会 `StackOverflowError` 崩游戏，建议加深度上限或载入期环检测。
- **javadoc 与行为不符**：`FlatExpressionWriter` 声称「任何回写结果重新解析都会得到同一棵表达式树」，实测不成立——负数字面量回读后被还原成 `subtract(0, v)` 形式（值相同、树不同，占 fuzz 样本的 34%），副带影响是顶层 `-2` 不再走 `Codec.either(DOUBLE, …)` 的数字分支而写成文本。
- `NamedFunction.name` 无字符校验，程序化构造含 `)` 的名字会写出不可回读文本（`$(a)b)`）。
- `InputFunction` record 构造器不校验非负（codec 用 `ExtraCodecs.NON_NEGATIVE_INT`），`InputFunction.of(-5)` 会静默取 0；`FlatExpressionParser.functionGetter` 与 `IFunction.getter` 是重复实现，可合并。
- `anvillib_math.mixins.json` 声明了 `required: true` 与包 `dev.anvilcraft.lib.v2.math.mixin`，但模块内不存在该包/任何 mixin（与 module.util 等空配置一致，可保留，确认下是否必要）。
### 🟢 看起来不错
- 表达式树统一为 `FunctionExpression` 一种节点（常量 / 输入 / 具名 / 内建 / 自定义都是调用），`Holder.direct` 内联 + `Holder.Reference` 引用两种形态清晰。
- 三种内联形式（数字 / flat 文本 / `function`+`arguments` 对象）的 `either`→`xor` 优先级正确，flat 写不出来时会正常回退到对象形式，不会丢数据。
- 内建函数在解析期做 arity 校验（`callChecked`），错误信息带位置；除零、负数开方、下标越界、名字未绑定都返回 `NaN`/`0` 而不抛异常，与 javadoc 一致。
- `Codec.lazyInitialized` + `IExpression.defer` 处理静态初始化顺序；`settings.gradle`、`gradle.properties`、`mods.toml`、`icon.png`、`jarJar` 列表（含 alphabetical 排序位置）都补齐了——只差 `modules.json`。
### 🧪 验证方式
无 Java 环境，按 skill 的等价转写 + 回环模糊测试法，把 `FlatExpressionParser` / `FlatExpressionWriter`（含 `parenthesize` / `number` / `isLiteralNumber` / `Operator.of`）逐行转写为 Python 后跑 `parse → write → parse`：
- 定向用例：`2*3`、`2.5*100`、`2*0.5`、`2.5*2.5`、`2*0.5`、`pow(-2,2)`、`multiply(2,-1)`、`x*20000000`、`x*0.0001` 全部复现（见上表）；
- 随机树 fuzz（N=4000，常量/输入/四则/幂/单参函数，取 3 组输入点求值比对）：**329 例值改变（8.2%）+ 19 例回读失败（0.5%）**，另有 1356 例仅结构不同（负号一元化）；
- 「值改变 + 回读失败」两类在修复 2/3/4 后应回到 0——建议补一组以 `FlatExpressionParser.parse → FlatExpressionWriter.write → parse` 的往返等价性单测（含 `const×const`、负数字面量、`|v|≥1e7`/`|v|<1e-3` 三个边界族）。
**结论: COMMENT（draft 阶段不阻塞）** — 模块骨架、注册表与 codec 分层设计清晰，但 **flat 文本回写路径存在可复现的值改变与不可回读缺陷**（`2*3` → `23`、`pow(-2,2)` → `-4`、`x*0.0001` 直接解码失败），且这条路径被 `IExpression.STREAM_CODEC` 用于网络同步，属于会静默污染存档/客户端数据的那一类；再加上 `modules.json` 缺项会让模块既不构建也不发布，建议在转正 (ready_for_review) 前先处理 🔴 1–4。
> 建议标题（本次 gh 未认证，无法自动修改）：`Add math module: expression parsing/evaluation library 新增数学模块：表达式解析与求值库`
---
*由 Hermes Agent 审查* |
|
💾 Self-improvement review: Skill 'text-codec-roundtrip-pr-review' created. · Patched references/pr-class-routing.md in skill 'github-webhook-pr-review' (1 replacement). |
Reason: shell execution via heredoc Reply |
Reason: script execution via -e/-c flag Reply |
|
已完成审查（diff 与 代码审查摘要 — PR #122
|
| 位置 | 状态 | 证据 |
|---|---|---|
.github/modules.json |
✅ | { "module": "math", "needs": ["util"] }，与三处枚举点交叉验证过：base 分支里只有 modules.json / settings.gradle / module.main 枚举模块 |
settings.gradle |
✅ | include 'module.math' + project(':module.math').name = 'anvillib-math-neoforge-1.21.1' |
module.main/build.gradle |
✅ | NOT_DEV / dev 两支都补了 jarJar(api(...))，位置在字母序内 |
generate-matrix.js 契约 |
✅ | module_id=anvillib-math、mod_id=anvillib_math 与 gradle.properties、jar 名一致 |
| gradle 依赖 ↔ needs | ✅ | module.math 只用到 v2.util.ISerializer（module.util），needs:["util"] 与 jarJar(implementation project(":anvillib-util-neoforge-1.21.1")) 一致 |
🔴 关键
FlatExpressionWriter.functionName()（第 248–254 行）+stripDefaultNamespace与解析器「anvillib 命名空间优先内建」规则冲突 → 静默改值。
写侧把anvillib:前缀一律省略；读侧FlatExpressionParser.parseIdentifier在 MAIN_ID 分支先LibBuiltInFunctions.byName(path)（第 442 行）再查注册表。因此数据包里注册的anvillib:<内建名>（如anvillib:sqrt、anvillib:max）永远无法按名引用，而且一个持有该函数引用的树回写后会被换成内建函数——这是本 PR 唯一会改变求值结果的往返缺口（对象形式 JSON → 编码 → 解码后语义变了），与类注释「任何回写结果重新解析都会得到同一棵表达式树」的承诺相悖。
我用逐行等价转写的 parser/writer 复现：节点anvillib:sqrt(4)→ 文本sqrt(4)→ 重新解析得到 builtin sqrt；而mymod:max（非 anvillib 命名空间）走「先查注册表」路径，正常往返 ✅。
建议（一行级）：functionName()里若key.location()在 MAIN_ID 且LibBuiltInFunctions.byName(path) != null而call.function().value()不是该内建，就返回null（退回对象形式），避免静默替换。可达性低（需要有人在anvillib命名空间注册撞名函数），但代价是静默错值，建议顺手补上。
⚠️ 警告
FlatExpressionWriter.negation(String)第 168 行的startsWith("-")早退，让「解析器读得进来的文本写不回去」。 实测（模型复现）：-(-0.5)、0-(-1)、x*(-(-2))、sqrt(-(-0.25))都解析成subtract(const 0, const -n)，回写时直接返回空 → codec 退回对象形式。该守卫的理由是「--x不是合法写法」，但negation(String)本身就无条件加括号，永远不可能写出--x；去掉守卫后-(-0.5)重新解析恰好得到subtract(0, const -0.5)（我用模型验证过子树结构逐节点相同）。影响：文本→树→文本不闭合（值不丢，只是表示形式退化；若下游依赖 flat 文本做规范写回/网络同步就会突然变成对象形式）。FlatExpressionTest.negationOfNegativeConstantFallsBackToObject目前把这个回退当成预期行为固定下来了，改的话需要同步更新该用例。IFunction.bind()（第 164–167、175–181 行）在「变参不在末位」时绑定错位 / 抛越界异常。 固定形参位的「列表不该出现在这里」检查按形参序号取values.get(fixed)，而values是按实参顺序排的（变参可能吞掉多个实参位），两者在变参非末位时错位：- 形参
["a","x...","b"]+ 调用f(1, $(xs...), 7)→ 误报「列表只能传给变参」； - 形参
["x...","b"]+ 调用f(1, $(xs...))→ 绑定循环走到values.get(2)IndexOutOfBoundsException（不是可读的校验错误）。
Parameters明确宣称支持变参在任意位置（BuiltInFunctionTest.variadicMayAppearAnywhere也断言了），但现有测试只覆盖了「变参在末位」和「变参首位的 forEach（它自己重写了apply）」。另外Parameters第 15 行注释「变参之后的形参永远取不到值」与bind()的variadicCount = total - fixedCount（后面固定形参确实会拿到值）自相矛盾，建议统一口径：要么校验循环改为按「第 i 个形参实际消费的实参下标」检查，要么在Parameters构造期就限制变参必须在末位，并同步文档/实现/测试。
- 形参
FlatExpressionParser.CACHE（第 57–58、116 行）的弱键达不到注释声称的回收效果，且无容量上限。 缓存值是表达式树，引用注册表函数的树里持有Holder.Reference，而Holder.Reference带一个owner（HolderOwner）字段——实测 MC 类文件里确认（net/minecraft/core/Holder$Reference.class有owner : Lnet/minecraft/core/HolderOwner;），注册表就是自己的 owner，而缓存键正是RegistryOps.getter()直接拿到的注册表实例 → 值强引用键，WeakHashMap 的弱键永远不可回收，每次数据包 reload 都会留下「旧注册表 + 该注册表下解析过的全部表达式」，与类注释「键用弱引用，换实例后旧缓存能被回收」不符；同时缓存没有条目上限，而parseValue是公开 API，下游用运行时拼接的文本反复调用会无界增长。建议：加容量上限（LRU），或在 reload 事件里清空，或把键换成不构成反向引用的标识。
💡 建议
- 字面量可以解析成无穷：
1e99999（指数 ≤ 8 位）经Double.parseDouble得到Infinity常量且不报错，而回写侧对非有限值一律拒绝 → 用户写的合法文本反而写不回去。建议解析期就拒绝非有限字面量。 Operator.UNARY（第 383 行）是死代码：Operator.of()从不返回它，needsParentheses也从不收到它，只活在注释里 → 删除或真正接进取负的括号判定。Parameters.bind(List<Double>)（第 161 行）全仓库无调用方（实际用的是IFunction.bind），且它把变参当末位处理（后面的固定形参会arguments.get(size)越界），属于会误导下游的公开 API → 删除或改为委托。NamedFunction与IExpression.Reference.Named是同一语义的两套节点（$(name)文本只解析成后者，只有对象形式能表达前者），测试里也得靠canonical()把它们折叠成同一个字符串才过得去 → 若没有刻意保留两套的正当理由，建议合并，减少「同一棵树两种表示」的认知负担。- 命名空间约定值得写进 javadoc：mod id 是
anvillib_math，但函数/类型都注册在anvillib（AnvilLibMath.MAIN_ID，与"type": "anvillib:custom"文档一致 ✅）。其它模块若也往anvillib:function_type/anvillib:function注册，需保证名字不冲突，建议在LibRegistries注释里点明这条约定。
🟢 看起来不错
- 上一轮评审报过的三类回写 bug 都已修复并有回归用例：常量并置（
isLiteralNumber排除带符号常量 +juxtaPositionable只放行字母/_/(/$，2*3不再写成23）、负底数（(-2)^2与-2^2语义分开）、指数记法（basicNumber十进制优先 +signedNumberLiteralAhead认回-5.0E-4）、负零符号位。 - 我用逐行等价转写的 parser + writer 模型跑了 60000 棵随机树（含极值/负零/指数常量、变参、取负、隐式乘法组合）：解析失败 0、结构改变 0、求值改变 0、回写不稳定 0；唯一「写不出来」的 13 例全部是上面
⚠️ -1 的「取负负常量」族。定向用例也确认 lambda（含多参/变参/嵌套/作为 forEach 末位实参）、mymod:bar这类数据包函数引用、anvillib:foo（不撞名）都能原样往返。 - 求值侧边界处理到位：除零 → NaN/∞、负数开方 → NaN、下标越界与未绑定名字取 0，都不抛异常，符合
IExpression.evaluate的 javadoc；CustomFunction有MAX_CALL_DEPTH = 64+ ThreadLocal 深度计数 +finally还原，自引用与互引用被拦成异常而非StackOverflowError（有测试）。 - 测试质量高：20000 轮随机树往返 + 一批「真实踩过的坑」定向断言，
MathTestBootstrap用最小注册表把表达式求值与游戏解耦，addModdingDependenciesTo(sourceSets.test)让单测在裸 JVM 上跑；math.fuzz.rounds可调。
📋 声称验证表
PR 描述为空，按模块功能对照 diff：
| 声称 | 状态 | 对应文件 |
|---|---|---|
flat 表达式解析器（x*2、2x、$(cost)、^、lambda） |
✅ | FlatExpressionParser(555) |
| 表达式回写成 flat 文本（规范形式） | ✅ | FlatExpressionWriter(411) |
| 表达式树 + 三支编解码（数字 / flat 文本 / 对象） | ✅ | IExpression, FunctionExpression |
| 函数类型注册表 + 数据包函数注册表 | ✅ | LibRegistries, LibFunctionTypes |
| 内建函数（四则/幂/单参/变参/forEach） | ✅ | LibBuiltInFunctions |
| lambda、变参、自定义函数、递归保护 | ✅ | LambdaFunction, Parameter(s), CustomFunction |
| 新模块 CI/发布/打包接线 | ✅ | .github/modules.json, settings.gradle, module.main/build.gradle |
| 单元测试 | ✅ | 4 个测试类 + MathTestBootstrap + MathFlatAssertions |
🧪 测试建议
| 被测目标 | 推荐场景 | 优先级 |
|---|---|---|
IFunction.bind |
变参不在末位 × 实参含 $(x...)：断言绑定结果与异常类型（当前会越界） |
🔴 |
FlatExpressionWriter.negation |
对「解析器产出的树」断言可写性：-(-0.5)、0-(-1) 的 parse→write→parse 闭合 |
🟡 |
FlatExpressionParser.CACHE |
连续两次 reload 后断言旧注册表可被 GC（弱引用探针） | 🟡 |
| 注册表撞名 | anvillib:<内建名> 数据包函数的往返语义（应退回对象形式或保真） |
🟡 |
| 字面量边界 | 1e99999 等超大指数字面量的解析结果（当前静默得到 Infinity） |
🟢 |
结论: COMMENT — 逻辑与测试都相当扎实，上一轮的回写 bug 已修复且被随机往返测试锁住；上面 4 条属于边界/健壮性问题，其中 🔴 会静默改值、
建议标题：
feat(math): add math expression library (parser / evaluation / function registry) 新增数学表达式库——当前标题Add math lib 添加数学库准确但偏笼统，可补充「表达式解析/求值」这一核心内容。本次因gh auth status显示 token 已失效（Active account: Gugle2308，token invalid）未自动修改，按约定不导出凭据，留由 maintainer 决定。
由 Hermes Agent 审查
|
💾 Self-improvement review: Patched SKILL.md in skill 'pr-diff-scope-reconciliation' (1 replacement). · Patched references/anvillib-math-pr122-findings.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). · Skill 'text-codec-roundtrip-pr-review' updated (full rewrite). · Patched SKILL.md in skill 'dependency-bump-pr-review' (1 replacement). |
🌿 Roseau API Breaking Change Report
|
Reason: Security scan — [HIGH] Pipe to interpreter: curl | python3: Command pipes output from 'curl' directly to interpreter 'python3'. Downloaded content will be executed without inspection. Reply |
|
审查完成。以下是审查意见（60k 轮独立 fuzz + 逐行等价转写验证）： 代码审查摘要 — PR #122操作: synchronize ✅ 验证手段（先给证据，再给结论）
🔴 关键
|
|
💾 Self-improvement review: Patched references/anvillib-math-pr122-findings.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). · Patched SKILL.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). |
🌿 Roseau API Breaking Change Report
|
|
审查完成。diff 与本地 代码审查摘要 — PR #122
|
| 上轮发现 | 本轮状态 | 证据 |
|---|---|---|
🔴 lambda 作二元操作数不补括号（2(x -> $(x)) → 2x -> $(x)） |
✅ 已修 + 有测试 | needsParentheses() 第 285 行对 LambdaFunction 返回 true；并置过滤第 196 行排除 lambda。本轮 lambda fuzz 五类全 0，LambdaTest.lambdaOperandsStayWritable 覆盖 4 个形状 |
clearCache() 全仓无调用点 |
✅ 已修 | 新增 LibCacheReloadHandler（ServerStartedEvent + OnDatapackSyncEvent）；对 NeoForge 源码确认 OnDatapackSyncEvent 的触发点确实含 /reload，首轮加载另有 ServerStartedEvent 兜住 |
LambdaFunction.apply 缺递归深度守卫 |
❌ 仍存（见 |
LambdaFunction.java 全文无 MAX_CALL_DEPTH；CustomFunction 仍是唯一带守卫的类型 |
💡 FLAT_OR_OBJECT_CODEC.encode 未检查强转 |
✅ 已修 | 改成 instanceof FunctionExpression + DataResult.error(...) |
💡 FOREACH.apply 直构绕过 arity 校验 |
✅ 已修 + 有测试 | FOREACH.apply 首行 this.parameters().checkArity(arguments.size())；forEachChecksArityOnDirectConstruction |
| 💡 解析期无嵌套深度上限 | 🟡 部分修复 | 新增 MAX_NESTING_DEPTH=512 + guardDepth，但 parseUnary 路径绕过（见 |
💡 测试盲区：randomTree 叶子无 lambda |
🟡 部分缓解 | 作者补了手写用例，但 MathFlatAssertions.randomLeaf 仍只有常量/输入/命名值三支，randomTree 仍显式排除 FOREACH（见 💡1） |
💡 死代码 Operator.UNARY / stripDefaultNamespace / Arguments.bound(String) |
✅ 全部已删 | 本轮 + 上轮提交一并清理；Parameters.bind(List<Double>) 也删了 |
💡 modules.json 条目顺序与列宽 |
✅ 已修 | math 已移到 multiblock 之前，needs 列与其他条目对齐 |
ⓘ NamedFunction 与 Reference.Named 双节点 |
ⓘ 仍存（未复报，仅记录） | flat 文本只产后者，测试仍需 canonical() 折叠 |
🔴 关键
1. FlatExpressionWriter.writableName() 只挡了「撞内建名」，没挡「撞输入变量名」→ 回写后静默改值
writableName()（FlatExpressionWriter.java:264-270）判断能否省略 anvillib 命名空间的唯一条件是 LibBuiltInFunctions.byName(path) != null。但解析器 parseIdentifier()（FlatExpressionParser.java:470-507）对裸标识符的解析顺序是：
variable(path) // x / y / z / x0 / x1 / x2 / x<digits> ← 第一步，且不要求后面跟 '('
LibBuiltInFunctions.byName(path)
FUNCTION_KEY 注册表
所以数据包在 anvillib 下注册一个名叫 x0（或 x/y/z/x12）的函数后，引用它的树会被回写成裸名，重解析却变成输入值 × 操作数：
注册函数 : anvillib:x0 (data/anvillib/anvillib/function/x0.json, type anvillib:custom)
树 : call(anvillib:x0, [const 5])
encode → 文本 : "x0(5)"
decode → 树 : multiply(<input 0>, 5) ← 结构+值双变，且不再指回那个函数
等价转写实测（writableName() 按当前实现镜像后逐名跑 write → parse）：
| 注册名 | 写出文本 | 重解析 |
|---|---|---|
anvillib:sqrt |
对象形式（已挡） | — ✅ |
anvillib:x / y / z |
x(5) / y(5) / z(5) |
<input 0/1/2> * 5 ❌ |
anvillib:x0 / x1 / x2 / x12 |
x0(5) … x12(5) |
<input 0/1/2/12> * 5 ❌ |
anvillib:collision-free |
collision-free(5) |
解析失败（见 🔴2） |
x0/x1/x2 是相当常见的命名，这条触发面比上轮那条内建名撞名更宽。影响链与上轮一致：FLAT_OR_OBJECT_CODEC.encode（IExpression.java:72-85）只要 write 成功就直接返回 flat 文本，不再写对象形式，所以坏文本会真的落进存档/网络包，decode 侧静默换值。
修法：在 writableName() 里把 parser 的变量规则也算上——path 形如 x/y/z/x<数字> 时返回 null（建议把 FlatExpressionParser.variable() 提成包内可见直接复用，避免两边规则各写一份再漂移）。
2. 函数名含标识符字符集之外的字符时「写得出、读不回」；本轮新增测试把这个缺口固化成了期望值
visitCall（FlatExpressionWriter.java:230-241）把 functionName() 的返回值原样拼进文本，而标识符扫描器 isIdentifierPart（FlatExpressionParser.java:603-605）只吃 [A-Za-z_][A-Za-z0-9_:.]*——- 与 / 都不在内（两者都是合法 ResourceLocation 路径字符）。
本轮新增的 CustomFunctionTest.builtInNameCollisionFallsBackToObject 第 258 行正是：
MathTestBootstrap.registerFunction("collision-free", ...); // data/anvillib/anvillib/function/collision-free.json，合法
assertEquals("collision-free(3)", MathTestBootstrap.writeFlat(...));该文本回读必然失败——等价转写用当前解析器复现：
parse("collision-free(3)")
→ expected '(' after function 'collision' at position 9 of expression "collision-free(3)"
（parseIdentifier 读到 collision 就停在 -，match('(') 失败。）第一轮提过的同类项「路径含 /」（data/<ns>/anvillib/function/utils/triple.json 这种常规写法）也仍未修。
由于 FLAT_OR_OBJECT_CODEC.encode 写得出就不回退，这条同样是写进数据后读不回来：作者测试只断言了文本、没有回读，于是 CI 全绿而缺口存活。
修法：writableName() 里加字符集校验——path（非 anvillib 命名空间则含 namespace: 的整体）必须全部落在 isIdentifierStart/isIdentifierPart 内，否则返回 null 退回对象形式；并把该测试的期望值改成 assertNull(writeFlat(...))，需要断言「省命名空间」时换一个合法名字（如当前同一测试里的 collision-free → collisionfree）。
建议补一条通用回归：对 writeFlat 的结果再跑一次 parseValue 并断言结构相同——本轮新加的几个用例（含这一条）只要接上 MathFlatAssertions.assertRoundTrip 就能立刻暴露。
⚠️ 警告
1. 本轮新增的嵌套深度上限在 parseUnary 的符号链路径上不可达
guardDepth() 只包在 parseLambda()（198-216）与 parsePower()（332-340）上，注释写的是「递归下降的入口都要先过这里」。但 parseUnary()（319-330）自身是直接递归、且不经过这两个入口：
if (this.match('-')) return this.call("subtract", ConstantFunction.of(0).call(), this.parseUnary()); // ← 每个 '-' 一帧
if (this.match('+')) return this.parseUnary();等价转写按同样位置插桩后的读数：
| 输入 | 结果 | 守卫计数最大值 | parseUnary 递归层数 |
|---|---|---|---|
"("*5000 + "1" + ")"*5000 |
nests too deeply（depth=513）✅ |
513 | 256 |
"1" + "^1"*5000 |
nests too deeply ✅ |
513 | 512 |
"-"*60000 + "1" |
parsed（守卫全程未触发） | 1 | 60000 |
"+"*60000 + "1" |
parsed | 2 | 60001 |
递归帧数与输入字符数 1:1，计数器在这条路径上永远到不了 512 ⇒ StackOverflowError（Error，parseResult 的 catch (RuntimeException) 拦不住）仍会从 codec 穿进数据包加载流程——正是这次加守卫想堵的那条链，只是换了入口。按注释里「2000 层括号就足以打穿默认栈」的同一口径，几千个 -/+ 量级的输入即等效。
修法：让 parseUnary 的符号递归也走 guardDepth，或改成先循环收集符号、解析完再逐层套 subtract(0, ·)——两处都是几行。
2. LambdaFunction.apply 仍无递归守卫（上轮
CustomFunction.apply 有 MAX_CALL_DEPTH=64 + ThreadLocal 计数（CustomFunction.java:44-98），LambdaFunction 没有。lambda 同样是 FUNCTION_KEY 的合法条目（数据包 JSON "type": "anvillib:lambda"，或下游 DeferredRegister），并且按名调用它会走 LambdaFunction.apply → body.evaluate(...)——于是注册两个 body 互调的 lambda（anvillib:a 的 body 为 b(1)，anvillib:b 的 body 为 a(1)）就能在不经过带守卫类型的情况下把 Java 栈打穿；同样的环如果写成两个 CustomFunction，会在 64 层抛出可读的 IllegalStateException。
修法：把上限搬到共用位置（IFunction 的 apply(Call) 默认实现，或 LambdaFunction.apply 里复用 CustomFunction 的那套计数），让「能注册进注册表、能互相引用」的节点类型受同一约束。
💡 建议
- fuzz 叶子集合仍未含 lambda（
MathFlatAssertions.randomLeaf只有常量 /InputFunction/NamedFunction，randomTree还显式排除FOREACH）。本轮修复只被 4 个手写用例覆盖；把 lambda（单参/变参）与FOREACH补进叶子集合，作者自带的随机往返测试就能覆盖这类「新语法节点 × 操作数位置」的组合——上一轮漏掉的正是这个维度。 - 三处注释/javadoc 已与实现不符：
FlatExpressionWriter.java:283-284写「因此一律退回对象形式」，实现是返回true让operand()补括号（行为是对的，注释会误导后续维护者）；MathFlatAssertions.java:58仍写「个别形状（取负一个负常量）没有合法的 flat 写法，这时编码退回对象形式」——该形状上轮已改成可写；IFunction.bind的@throws只列了「实参个数不符 / 列表用在固定形参位」，本轮新增的「$(x...)铺开的实参多于该位所需」也抛IllegalStateException，建议一起写上。
IFunction.java:183-190的未检查强转：((IExpression.Reference) arguments.get(argument)).name()。当前安全（Many只可能来自Spread），但这份安全依赖的是别处的不变量；改成instanceof/把名字随Value一起携带更稳。clearCache()挂在OnDatapackSyncEvent上会在每个玩家进服时清空缓存（该事件在进服与/reload都会触发）。功能上无害，但每次进服都会丢掉整表缓存；按event.getPlayer() == null过滤成「仅全员同步/重载」更贴合意图。- （记录）
NamedFunction与IExpression.Reference.Named仍是同一语义的两套节点，flat 文本只能产出后者——两轮未动，仅供后续决定是否合并。
🟢 看起来不错
- 上轮的 🔴/
⚠️ /💡 基本全清，且每一项修复都配了定向回归用例（lambdaOperandsStayWritable、negationOfNegativeConstantStaysWritable、nonFiniteLiteralRejected、tooDeeplyNestedIsRejected、forEachChecksArityOnDirectConstruction、builtInNameCollisionFallsBackToObject、variadicNotLastBindsByConsumptionOrder），回归读数也从「1636 parse-fail」变成全 0。 needsParentheses的修法同时兼顾了两侧：补括号 + 并置过滤，且注释解释了「->绑得比所有运算符都松」，2(x -> $(x))这类最危险的并置吞并路径被单独拦住。- 缓存治理这次落到实处：
CACHE_CAPACITY=512的 LRU + 主动clearCache()+ 事件挂钩，javadoc 也把「值强引用键 ⇒ 弱键回收不掉」的原因写清楚了。 bind的形参绑定改成「按形参逐个take」并补上「铺开过多」的可读报错，Parameters注释与实现的自相矛盾也一并改对。- 死代码清理干净（
Operator.UNARY、stripDefaultNamespace、Arguments.bound、Parameters.bind），modules.json顺序/对齐、IExpression.encode的DataResult.error、FOREACH直构 arity 校验都到位。 - 新模块接线完整（
settings.gradle/gradle.properties/mods.toml/mixins.json/icon.png/module.main的 jarJar /.github/modules.json的needs:["util"]），本轮 diff 仍能全量 apply、CI 无 ghost。
🧪 测试建议
| 被测目标 | 推荐场景 | 优先级 |
|---|---|---|
FlatExpressionWriter.writableName() |
表驱动：anvillib:{sqrt,x,y,z,x0,x12,collision-free,utils/triple}，断言不可写的返回 null（现在只断言了文本） |
🔴 |
writeFlat → parseValue 回读不变量 |
把新增用例接上 MathFlatAssertions.assertRoundTrip，断言写出文本能读回同一棵树 |
🔴 |
FlatExpressionParser.parseUnary() 符号链 |
"-".repeat(N)+"1"、"+".repeat(N)+"1" 应报 nests too deeply 而非 StackOverflowError |
|
LambdaFunction.apply |
两个 body 互调的注册 lambda → 应抛可读错误；与 CustomFunction 的 64 层行为对齐 |
|
IFunction.bind 变参/列表 |
已覆盖 [a,x...,b] 与 [x...,b]；建议再补「$(xs...) 恰好用尽」「两个 spread 拼满变参位」 |
💡 |
MathFlatAssertions.randomLeaf |
叶子集合加入 lambda 与 FOREACH，让自带随机往返覆盖操作数位置 |
💡 |
结论: REQUEST_CHANGES — 本轮修复方向、质量与测试配套都很好（lambda 操作数、缓存生命周期、深度上限、CCE、arity、死代码清理全部落实，回环 fuzz 由「1636 例读不回」变为全 0）。剩下的两条 🔴 都属于「encode 写得出来、decode 读不回/读错」的静默数据改变，恰好是 FlatExpressionWriter 类注释承诺的那个不变量，且修法都集中在 writableName() 一处；
标题：gh auth status 报 token invalid（Active account 存在），按 webhook 规矩未执行 gh pr edit；建议标题 Add math module: flat expression parsing & evaluation 新增数学模块：表达式解析与求值库（本次未自动修改）。
由 Hermes Agent 审查
|
💾 Self-improvement review: Patched SKILL.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). · Patched references/anvillib-math-pr122-findings.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). |
🌿 Roseau API Breaking Change Report
|
Reason: recursive delete Reply |
Reason: script execution via -e/-c flag Reply |
代码审查总结结论：已审查 💬（草稿 PR；无关键问题，1 个警告 + 6 个建议，均不阻塞；草稿状态下即可讨论） PR： #122 — Add math lib 添加数学库 审查范围与取证
|
| 被测目标 | 推荐测试场景 | 优先级 |
|---|---|---|
parseIdentifier 注册表分支 / IFunction.bind |
数据包函数实参个数不符时，断言在 parseValue 期就抛错（对应建议 1） |
🟡 |
FlatExpressionWriter.writableName |
注册 mymod:max、mymod:sqrt 后断言 writeFlat 写出 mymod:max(...) 且能读回（对应建议 2） |
🟡 |
FlatExpressionParser.guardDepth |
嵌套边界："f("*180+")"*180 应报 nests too deeply、170 层应通过，钉住 512 常量的实际语义 |
🟢 |
CACHE / clearCache |
用两个不同 RegistryAccess 解析同一段文本，断言 CACHE 不随注册表实例累积、clearCache 后旧注册表可被 GC（对应建议 3） |
🟢 |
标题： 现标题「Add math lib 添加数学库」已是「英文 + 中文」格式，无需修改。（另：本机 gh auth status 为未认证状态，即使需要改标题也无法执行 gh pr edit——如需调整请人工处理。）
由 Hermes Agent 审查
|
💾 Self-improvement review: Patched SKILL.md in skill 'hermes-approvals' (1 replacement). |
🌿 Roseau API Breaking Change Report
|
Reason: Security scan — [HIGH] Pipe to interpreter: curl | python3: Command pipes output from 'curl' directly to interpreter 'python3'. Downloaded content will be executed without inspection. Reply |
|
💾 Self-improvement review: Patched SKILL.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). · Patched references/anvillib-math-pr122-findings.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). |
🌿 Roseau API Breaking Change Report
|
Reason: recursive delete Reply |
代码审查摘要 — PR #122操作: 新模块 🟢 新模块接入检查表（本 PR 无一遗漏）
🔴 关键
|
| 被测目标 | 推荐场景 | 优先级 |
|---|---|---|
parseNamed / IExpression.CODEC 编码 |
$(cost-1)、$(a b)、$(成本)、$(...) 解析后再 encode，断言不抛异常（当前返回 DataResult.error） |
🔴 |
IExpression.STREAM_CODEC |
RegistryFriendlyByteBuf 往返：常量 / 文本 / 对象三支 + 含 Reference 的树 |
🔴 |
LibBuiltInFunctions.FOREACH |
$(typo...) 未绑定成列表时期望报错而非返回 0 |
🟡 |
IFunction.HOLDER_STREAM_CODEC |
Holder.direct 内联定义与注册表引用的编解码对称性 |
🟡 |
FlatExpressionParser.error |
超长原文只截取报错片段（日志膨胀回归） | 🟢 |
由 Hermes Agent 审查
|
💾 Self-improvement review: Patched SKILL.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). · Patched references/anvillib-math-pr122-findings.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). |
代码审查摘要 — PR #122
|
| 声称 | 状态 | 对应实现 |
|---|---|---|
Add math lib / 添加数学库 |
✅ | 新模块 module.math：表达式树（IExpression/FunctionExpression/Arguments）、flat 文本双向编解码（FlatExpressionParser 719 行 / FlatExpressionWriter 613 行）、内建函数与数据包函数注册表、lambda、7 个测试类 |
| （附带）新增模块接入构建/发布 | ✅ | modules.json、settings.gradle、module.main/build.gradle、CI 动态矩阵无需改动 |
标题已符合 <英文> <中文> 格式且描述准确，未修改。
🧪 测试建议
| 被测目标 | 推荐测试场景 | 优先级 |
|---|---|---|
IFunction.bind |
Parameters.parse(["x...","b"]) + $(a...) 绑定为空列表落在变参位（当前误报，见 Many 长度超过该位剩余需求的报错文案 |
🔴 |
IExpression.STREAM_CODEC / FunctionExpression.STREAM_CODEC / IFunction.HOLDER_STREAM_CODEC |
RegistryFriendlyByteBuf 往返：内建函数（Holder.direct）、数据包函数引用、含 lambda/custom 的树 |
🔴 |
FlatExpressionWriter#write |
自校验失败时确实退回对象形式：编码结果不是字符串原语（如错误 arity 的程序化构造树） | 🟡 |
FlatExpressionParser 缓存 |
512 上限淘汰行为；clearCache() 后旧注册表不被引用 |
🟡 |
Parameters#checkArity(List) |
$(x...) 落在无变参函数的固定位（sqrt($(x...))）应报「按非铺开个数」——已有测试，建议再加「两个铺开实参」边界 |
🟢 |
Arguments#isList |
未绑定名字 vs 绑定空列表的区分（已有测试，保持） | 🟢 |
结论: COMMENT — 设计与实现质量很高（回写自校验 + 解析期校验 + 深度守卫 + 缓存生命周期都很到位），构建/CI 接入无遗漏；无阻塞问题。建议合并前处理 bind 空铺开误拒，附一条测试）与 build-l2 (math, ...) 与 roseau-l2 两个作业通过再合并。
由 Hermes Agent 审查
|
💾 Self-improvement review: Patched SKILL.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). · Patched SKILL.md in skill 'ci-workflow-pr-review' (1 replacement). · Patched references/head-sha-freshness-and-prior-review-verification.md in skill 'github-webhook-pr-review' (1 replacement). |
🌿 Roseau API Breaking Change Report
|
🌿 Roseau API Breaking Change Report
|
Reason: recursive delete Reply |
Reason: script execution via -e/-c flag Reply |
|
All prior findings verified as fixed; diff integrity confirmed (41 files / +6091 / −0, matching the API exactly). Writing the review. 代码审查摘要 — PR #122
|
| 上轮发现 | 状态 | 证据（当前 head） |
|---|---|---|
IFunction.bind 空列表 $(x...) 落在「变参之后的固定形参位」被误拒 |
✅ 已修 | IFunction.java:262-265 新增「变参自己空着时推游标」；234-238 补 argument >= values.size() 前置检查 |
minimumCount()==0 矛盾 |
✅ 已修 | Parameter 类注释改为「变参可以是零个实参」；MIN/MAX 改为「取不到值时返回 0」 |
💡 juxtaPositionable 注释漂移（仍写 2$(a)） |
✅ 已修 | 注释改为「右侧只能是标识符或括号；$(name) 一律显式写 *」 |
| 💡 网络 stream codec 路径零覆盖 | ✅ 已修 | 新增 StreamCodecTest（9 用例：Holder.direct 内建、数据包函数引用、lambda/custom 整树、字面量/对象两支、字节稳定性、holderRegistry 直柄） |
💡 write 未点明自校验的编解码开销 |
✅ 已修 | write javadoc 新增「代价：O(2×) 树规模…高频路径应缓存编码结果」 |
💡 .gitignore 加 logs/ 属范围外 |
⬜ 仍在 | 可选，不阻塞 |
bind 的新逻辑我按位置组合逐位核对过，未发现新漏洞：["x...","b"]+空 $(a...)→x=[],b=1；["a","x...","c"]+空 $(e...)→a/x=[]/c 各就位；["b","x..."]+[$(a...),…] 也正确。variadicCount 不可能为负（checkArity(total) 先跑，minimumArity()==fixedCount）。
⚠️ 仍待处理
1. LibBuiltInFunctions.FOREACH 漏用 Arguments.isList（前轮已报、本轮未改）
LibBuiltInFunctions.java:177-178 仍自行解析铺开实参：
if (argument instanceof IExpression.Reference.Spread(String name)) {
values.addAll(inputs.list(name)); // 名字拼错/未绑定 → 静默空列表而 Arguments.list() 对「没绑定的名字」和「绑定成空列表的变参」都返回空（正是 Arguments.java:83-91 里 isList 存在的理由，IFunction.java:210-213 也守着这条）。于是 forEach($(typo...), x -> $(x)) 静默返回 0，而同一个拼写错误在 min($(typo...)) 会抛出可读的 $(typo...) is not bound to a list。测试里 unboundSpreadNameIsReportedClearly 只覆盖了 min（走 bind）这条路径，forEach 没有对应用例。建议在 177 行前加同样的 inputs.isList(name) 守卫并补一条 forEach 用例。
2. 〔新〕variable() 会抛异常，但写侧调用方把它当布尔/可空判断用
FlatExpressionParser.variable()（620-627）对 x + 溢出 int 的数字名抛 IllegalArgumentException("Input index is too large")，而它的两个写侧调用方并不期待异常：
isWritableFunctionName（693）声明返回boolean：数据包注册一个anvillib:x3000000000这类函数后，writableName→functionName→visitCall→bare→FlatExpressionWriter.write第 62 行（在try之外，try从 65 行才开始），异常会直接穿透FlatExpressionWriter.write→FlatExpressionParser.codec().encode→Codec.encode。而FLAT_OR_OBJECT_CODEC.encode是靠encodeStart返回失败DataResult才退回对象形式的——异常一来，「写不出来就退回对象形式」这条契约就断了，栈会一直抛到调用方（存档saveAdditional/ 网络同步路径）。触发面很窄（需要数据包函数命名为anvillib:x<数字>且数字 > 2³¹−1），但修起来很便宜。resolveReference（FlatExpressionWriter.java:131）：同一名字的NamedFunction会被sameMeaning抛异常 → 被write的catch吞掉 → 永远退回对象形式（Reference.Named因为 record 相等会提前返回，不受影响）。
对照：解析路径没这个问题——parseResult 的 catch (RuntimeException) 把同样的异常转成了 DataResult.error。建议二选一（推荐都做）：(a) 让 variable() 对溢出下标返回 null，把「Input index is too large」留在 parseIdentifier 的解析路径上；(b) 把 bare(...) 也放进 write() 的 try，让写侧任何 RuntimeException 都降级为对象形式而不是穿透 codec。
💡 建议
3. README 未更新，且现在已经与实现不符（前轮已报、未改） — README.md 是模块仓库的对外入口，三处都缺 math：① 14-27 行的模块表格（Config…Wheel 共 12 项）；② 「模块介绍」分节（无 Math 一节）；③ 319-322 行「anvillib-neoforge-1.21.1 为聚合发行模块，默认打包并重导出以下子模块」的清单——本 PR 已在 module.main/build.gradle 里 jarJar(api(…anvillib-math-neoforge-1.21.1…))，这份清单因此由「不完整」变成了「不准确」，建议一并补上 math 条目与依赖坐标示例。
4. Math.round 两处边界语义 — evaluateInt（IExpression.java:103-105）与内建 ROUND（LibBuiltInFunctions.java:107-111）：Math.round(NaN) == 0（round(sqrt(-1)) → 0 而非 NaN）、超出 long 范围时饱和（round(1e300) → 9.223372036854776E18）、(int) 强转还会回绕（evaluateInt(3e9) → -1294967296）。模块里其它函数（divide/sqrt）都刻意保留 NaN/Inf 语义，这两处值得在 javadoc 里点一句或显式 clamp。
5. Parameter/Parameters 形参名字符集（前轮 #33 的 (c) 建议，未改）——Parameter（canonical ctor）只拒绝空名与含 ... 的名字，CustomFunction.named(List.of("a b"), …)、LambdaFunction.named(List.of("x)"), …) 都构造得出来。当前不会写坏数据（isWritableReferenceName / bare / 回写自校验三重兜底都会退回对象形式），只是给下游留了一类必然写不出文本的静默形状；从源头限制成标识符字符集可省掉这条路径。
🟢 看起来不错
- 回写自校验仍然是本 PR 最稳的一环：
write()返回前实测「再解析 → 再写 → 文本完全相同 +sameMeaning」，任一不成立即返回空并退回对象形式；juxtapositionReadsBack实测并置可读回（挡2*e1(x) → 2e1(x)）、text.startsWith("-")补括号、BigDecimal#toPlainString+trimTrailingZeros处理1.0E-7/0.0001边界、负零保符号位、isWritableFunctionName/isWritableReferenceName读写共用一份判断。我特意检查了JUXTAPOSING/VERIFYING两个 ThreadLocal 短路是否有「跳过预检 ⇒ 写出坏文本」的缝隙：由于接受前必须bare(parseValue(text)).equals(text)，更宽松的那次（短路）只要与保守那次（已预检）文本不同就会被拒，取不到坏结果。 StreamCodecTest是真验证（比较形状 + 求值结果，不是只断言非空），补上了上轮点名的网络路径空白；ParseCacheTest把「同一 registry 命中、clearCache失效、超 512 淘汰、asLookup稳定性」都钉住了。- 构建/CI 接入完整无遗漏：
.github/modules.json新增{ "module": "math", "needs": ["util"] }（落在 level 2，build-l2/roseau-l2存在）、settings.gradle的include+ 重命名为anvillib-math-neoforge-1.21.1、module.main/build.gradle两条分支（NOT_DEV走latest.release/ dev 走project(...)）都成对补齐；generate-matrix.js推导出的anvillib-math/anvillib_math/module.math与包名、archivesName、mods.toml完全对得上。@EventBusSubscriber(modid=…)不写bus=的写法与module.recipe、module.multiblock、module.util(ClientTickRecorder, 游戏总线事件) 一致，不是遗漏。测试已挂进build→check→test，CI 会真跑到。 - 缓存生命周期：按注册表分组 + 上限 512 +
/reload（OnDatapackSyncEvent#getPlayer()==null，我对着 NeoForge 源码确认了该语义）与客户端断连清理，javadoc 还诚实写明「弱键因值强引用键而回收不掉，存活期是手动的」。
📋 声称验证表（PR body 为空，按标题核对）
| 声称 | 状态 | 对应文件 |
|---|---|---|
flat 表达式解析（x*2/2x/$(name)/^/一元/隐式乘法/×÷·/lambda） |
✅ | FlatExpressionParser |
| 反向回写（规范化文本 + 自校验 + 对象形式回退） | ✅ | FlatExpressionWriter、IExpression.FLAT_OR_OBJECT_CODEC |
| 函数类型 / 数据包函数双注册表，支持下游扩展 | ✅ | LibRegistries（function_type、function）、LibFunctionTypes、LibBuiltInFunctions |
| 内建函数集（四则/乘方/abs/floor/ceil/round/sqrt/min/max/foreach） | ✅ | LibBuiltInFunctions（14 个） |
| 值/表达式三种内联形式（数字 / flat 文本 / 对象） | ✅ | IExpression.CODEC |
| 测试与构建接入 | ✅ | 7 个测试类（~2000 行）+ modules.json/settings.gradle/module.main/build.gradle |
结论: COMMENT（无阻塞项） — 上一轮全部发现已修好并各自补了测试，CI 的 math 构建已转绿，回写不变量与求值/解析校验都很扎实，我没有发现会静默写坏数据的路径。上面
建议标题： Add anvillib-math module: flat math expression parser/writer 添加 anvillib-math 数学表达式模块（当前 Add math lib 添加数学库 格式已合规，只是没体现模块名与「解析/回写」这个核心能力；本次 gh auth status 显示 /opt/data/home/.config/gh/hosts.yml 的 token 已失效，为免触发凭据类操作未自动修改）。
由 Hermes Agent 审查
|
💾 Self-improvement review: Patched references/anvillib-math-pr122-findings.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). · Patched SKILL.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). · Patched references/duplicate-review-detection-and-stop.md in skill 'github-webhook-pr-review' (1 replacement). |
🌿 Roseau API Breaking Change Report
|
Reason: script execution via -e/-c flag Reply |
代码审查摘要 — PR #122操作: synchronize（标题： 📋 声称验证表（README Math 章节 vs 代码）
🔴 关键（合并前处理）
|
| 被测目标 | 推荐场景 | 优先级 |
|---|---|---|
FlatExpressionParser.parseValue 链式文本 |
"1+".repeat(20000)+"1" → 断言按预算报错，或断言 write/evaluate 不 SOE |
🔴 |
IExpression.CODEC 对象形式 |
手写 {"function":{"builtin":"add"},"arguments":[1]} decode → 应返回失败 DataResult |
🔴 |
IExpression.STREAM_CODEC 高频路径 |
同一棵树连续 encode 两次，确认无累积成本（配合 4 的缓存改动） | 🟢 |
结论: REQUEST_CHANGES — 唯一阻塞项是那 10 个 logs/*.log.gz 二进制文件（一旦合并会永久留在 git 历史）；代码本身完成度和测试质量都很高，第 2、3 条建议同期处理，其余为非阻塞改进。
由 Hermes Agent 审查（未编译/未运行测试：本环境无 JDK，结论基于源码逐行核对与跨分支交叉验证）
|
💾 Self-improvement review: Patched SKILL.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). |
代码审查摘要 — PR #122操作: synchronize（PR 标题为 🔴 关键1. 解析器接受的 这是本 PR 自己反复强调的不变量被打破的唯一位置，且是真实可达的：
后果：下列文本都能被解析、能求值，但永远编码不出去（
对比：现有测试 建议（二选一或都做，改动都很小）：
|
| 声称 | 状态 | 证据 |
|---|---|---|
flat 语法 x*2、2x、x^2、$(name)、$(name...)、x -> $(x)*2，乘号 */×/· |
✅ | parseMultiplicative 的 match('*'|'×'|'·')、parseNamed、parseLambda |
| 反向回写并「自校验再解析、再写文本完全相同」 | ✅ | FlatExpressionWriter.write() |
| 写不出来自动退回对象形式，不写出读不回文本 | 对 FunctionExpression 成立；Reference（解析器产出）无对象形式 → 见 🔴 |
|
双注册表（function_type / function）对下游开放 |
✅ | LibRegistries + LibFunctionTypes / DataPackRegistryEvent |
内建函数：四则 + pow/abs/floor/ceil/round/sqrt/min/max/foreach |
✅ | LibBuiltInFunctions 13 个枚举，无缺无多 |
| 自定义函数变参下限 0、可不在末位 | ✅ | Parameter.minimumCount()、IFunction.bind() 的配位逻辑与「空变参推游标」分支 |
| 三种内联形式（数字 / flat 文本 / 对象）自动选择 | ✅ | IExpression.CODEC = either(DOUBLE, FLAT_OR_OBJECT)，常量优先写数字 |
新模块被 CI 构建（modules.json、settings.gradle、module.main） |
✅ | 三处均已补；但 Roseau 评论表漏 math → 见 |
结论: REQUEST_CHANGES — 代码质量与测试深度都远超同类模块 PR，绝大部分「parser/writer 回环」风险已被自校验与 fuzz 覆盖；但 🔴 那条是契约级漏洞（解析接受却永远无法序列化，Reference 无对象形式兜底），修复量只有几行（parseNamed 校验载荷，或让 Reference 也能降级），建议修掉后再合并。
|
💾 Self-improvement review: Patched SKILL.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). |
|
Review complete. Here's my assessment: 代码审查摘要 — PR #122
|
| 检查项 | 状态 | 证据 |
|---|---|---|
| 新模块注册进 CI 矩阵（高频遗漏项） | ✅ | .github/modules.json 增 { "module": "math", "needs": ["util"] }；generate-matrix.js 拓扑分层后 math 落 level_2 |
settings.gradle include + 项目重命名 |
✅ | include 'module.math' + project(':module.math').name = 'anvillib-math-neoforge-1.21.1'（与 module.main 的 project(":anvillib-math-neoforge-1.21.1") 对齐） |
| 聚合模块 jarJar 接入 | ✅ | module.main/build.gradle NOT_DEV 走 latest.release、dev 走 project(...)，两侧都补齐 |
| 构建属性完整（无缺 expand 属性） | ✅ | module.math/gradle.properties 只放 mod_id/name/description，mod_version/mod_group_id/mod_license/range 均继承根 gradle.properties |
| mod 元数据与源码一致 | ✅ | mods.toml modId = ${mod_id} = anvillib_math；@Mod(MOD_ID)、@EventBusSubscriber(modid = MOD_ID) 一致；mixins.json 空配置与 module.util 既有约定一致（JAVA_8 / anvillib.refmap.json） |
| 注册命名空间一致性 | ✅ | 类型注册在 MAIN_ID（anvillib），与 CustomFunction javadoc 的 "type": "anvillib:custom"、README 的 anvillib:function 路径一致 |
| 文档同步 | ✅ | README 模块表/重导出清单/依赖示例都补了 math（顺带补齐了 collision/explosion/font/rpc/space-select/sync） |
🔴 关键
未发现阻塞合并的问题。回写路径（parser + writer）的经典坑位全部命中并已被机制性防住：常量并置（2*3→23）、负字面量（pow(-2,2)→-2^2）、Double.toString 指数形式（1.0E-4）、负零丢符号位、2*e1(x)→2e1(x) 被指数记法吃掉 —— 而且 FlatExpressionWriter.write() 在返回前做「再解析 + 再写 + sameMeaning」自校验，任何一处不成立就退回对象形式，这条设计把「写得出来但读不回去」的风险从「静默损坏存档」降级为「退化到对象形式」。
⚠️ 警告
-
FlatExpressionParser.parseIdentifier/IFunction.bind— 解析期放行、求值期抛出的形状可以从数据包 JSON 构造出来
IExpression.evaluate的 javadoc 承诺「除零、负数开方、下标越界、名字未绑定等情况不会抛出异常」，但以下形状能被IExpression.CODEC正常解码，直到求值才抛：- 顶层
$(x...)（Reference.Spread.evaluate抛IllegalStateException）； $(x...)落在固定形参位：Parameters#checkArity(List)在有变参形参时一律放行（文档写的是「真实长度由IFunction#bind再判一次」），于是形如f($(x...), 1)（f声明(a, x...)）解码成功、IFunction.bind抛IllegalStateException；forEach末位不是 lambda（同样只在apply里判）；- 自引用 / 两个注册 lambda 互相引用（
guarded的MAX_CALL_DEPTHISE——这条是本意的反崩栈护栏）。
下游 BE 通常直接在 tick 里调
evaluate()，这些异常没有捕获点就会打到 tick 循环。建议：把「会抛的那部分」写进evaluate/evaluateInt的 javadoc（现在读起来像「求值不抛」），并在 README 的接入示例里点明数据包作者写错时的表现。深度守卫那一条无需改动。 - 顶层
-
函数名大小写两侧规则不对称 — 读入方向存在静默遮蔽
parseIdentifier先toLowerCase(Locale.ROOT)再查注册表（withDefaultNamespace(lower)、function(lower, name)），而FlatExpressionWriter.writableName用注册路径原样写出。- 读入方向：
mymod:Max会被折成mymod:max去查表，查不到再退回内建byName("max")—— 于是数据包里注册的mymod:Max在 flat 文本里被内建max静默顶替（求值结果可能完全不同），且没有任何回写自校验会经过这条读路径。 - 写回方向：
mymod:MyFunc/anvillib:MyFunc这类含大写的路径永远写不出 flat 文本（writableName判定通过、解析侧读不回 → 自校验兜底退回对象形式，不损坏数据，但 flat 文本这条快路径永久失效）。
建议二选一：
writableName/isWritableFunctionName直接拒绝非全小写名字（规则显式化、报错可读），或解析侧不再折叠大小写。当前状态是「读放宽、写收紧」，缺少文档说明。
💡 建议（非阻塞）
IFunction.CODEC/IFunction.STREAM_CODEC/IExpression.LIST_CODEC全 PR 无调用点、无测试：公开 API 留给下游可以理解，但IFunction.CODEC的自定义编码器带lookup.listElements()全表线性扫描 +input.equals(ref.value())的等值归一语义，一旦下游在存档/同步路径上用它就是 O(#函数)/次，而目前无人验证。至少补一个「注册条目等值时归一为引用」的用例（StreamCodecTest已经覆盖了HOLDER_STREAM_CODEC，这份可以对称补上）。另外FunctionExpression.MAP_CODEC里内联的IExpression.CODEC.listOf()与新增的IExpression.LIST_CODEC重复，建议统一引用后者。IExpression.FLAT_OR_OBJECT_CODEC.decode丢掉了 flat 侧的报错：flat 解析失败后直接改报对象形式的失败信息，数据包作者拼错文本时看到的是Expected map-like object...，而不是带位置信息的Invalid expression: ... at position 10 of expression "..."。建议把 flat 的错误合并进最终DataResult.error，调试成本差别很大。parseNumber指数长度上限（≤8 位）超限时的回退：this.position = mantissaEnd后2e123456789会落到标识符分支，报expected '(' after function 'e123456789'—— 与真实原因（指数过大）无关。5 位指数的用例已被nonFiniteLiteralRejected钉住，超长指数建议同样报number out of range。- CI 每次
:anvillib-math-neoforge-1.21.1:build都会跑 4 × 20000 轮 fuzz（math.fuzz.rounds默认 20000），若构建时间被拉长可考虑调低默认值；另外ci.yml里deploy-l2与main同层并行，而 main 在 NOT_DEV 下解析anvillib-math-...:latest.release，本模块首次发布后那一轮 CI 存在 main 早于 deploy 取件的时序风险，值得留意一次。
🟢 看起来不错
- 回写自校验（写→解析→再写 +
sameMeaning归一NamedFunction/Reference.Named/InputFunction）是这份实现的核心价值，把整类回环 bug 从「静默改值」变成「退回对象形式」； - 双递归护栏（
MAX_NESTING_DEPTH=512计数上限 +IFunction.MAX_CALL_DEPTH=64），并且把「512 计数 ≈ 169 层实嵌套」这个反直觉边界用测试钉住了 —— 这一步做得比大多数库都细； - 解析缓存：容量上限 + accessOrder LRU + 弱键指回注册表的问题在 javadoc 里点明，并挂好了
ServerStartedEvent/OnDatapackSyncEvent（getPlayer()==null）/ClientPlayerNetworkEvent.LoggingOut三处清理钩子，weak-hashmap那类泄漏被正面处理了； Arguments.withAll+IFunction.bind的变参配位（变参任意位置、空变参时游标仍要推进）逻辑有专门的回归用例；- 写不出文本时一律
Optional.empty()并靠DataResult.error走对象形式，writeSideFailuresNeverEscapeTheCodec这组用例把「异常不得穿透IExpression.CODEC」这条契约钉死了 —— 这是库代码最容易漏的一条。
🧪 测试建议
| 被测目标 | 建议场景 | 优先级 |
|---|---|---|
MathFlatAssertions.randomTree |
随机树叶子/算子目前不含 Reference.Spread、CustomFunction、注册函数（仅 e1 系列单独覆盖）；建议加「变参实参位 + Spread」的随机维度 |
🟡 |
IFunction.CODEC / IFunction.STREAM_CODEC |
注册条目等值 → 归一为引用的编码用例；Holder.direct → 注册引用的网络往返 |
🟡 |
FlatExpressionParser.parseIdentifier |
大写路径（mymod:Max 注册后能否被 flat 文本正确解析）——把上面 |
🟢 |
结论: COMMENT — 无阻塞项。工程质量很高（自校验回写 + 双递归护栏 + 8 个测试文件 + CI 接入齐全），上面
建议 PR 标题
Add math module: serializable math expression system 新增 Math 模块：可序列化的数学表达式系统
（gh 当前未认证，gh pr edit 无法执行 —— 未自动修改标题；本次审查流程用 curl 公开 diff，未使用任何 token。）
由 Hermes Agent 审查
|
💾 Self-improvement review: Patched SKILL.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). · Patched references/anvillib-math-pr122-findings.md in skill 'text-codec-roundtrip-pr-review' (1 replacement). |
|
PR #122 状态为 closed，按 Webhook 审查规则（closed/labeled → 停止，不发布审查意见），本次不做代码审查，也不输出审查评论。 如需重新审查，请在该 PR 重新打开或推送新提交后触发。 |
3 participants
No description provided.