Skip to content

feat: allow Recall operators to write common fields (mutating semantics) - #167

Merged
Liam0205 merged 4 commits into
masterfrom
feat/recall-set-common
Jul 16, 2026
Merged

feat: allow Recall operators to write common fields (mutating semantics)#167
Liam0205 merged 4 commits into
masterfrom
feat/recall-set-common

Conversation

@Liam0205

Copy link
Copy Markdown
Owner

Summary

Removes the SetCommon restriction from Recall's ValidateOutput across all three engines, allowing Recall operators to emit request-level common state (e.g. a recall-generated request id) that downstream operators consume.

Motivation

The previous design forbade Recall from writing common fields. This meant recall-produced metadata (request ids, trace ids, source identifiers) had no legitimate exit — it either got stuffed into every item row (semantically wrong, N-fold redundant) or was discarded. The restriction existed for DAG dependency simplification, but investigation confirmed the DAG layer already builds correct edges from CommonOutput regardless of operator type — the restriction was purely a runtime assertion with no structural justification.

Design

Recall's common write is a normal mutating hazard participant in the DAG common pass:

  • Two recalls writing the same common field → WAW serialized (deterministic)
  • Two recalls writing different common fields → item additive parallelism preserved
  • Downstream common reader → automatic RAW edge from the recall

This was verified by a three-case probe (now deleted — pure investigation, not production test) before any code change. The DAG addEdges function processes CommonOutput without inspecting operator type.

Still forbidden: SetItem, RemoveItem, SetItemOrder. Recall's only item action remains AddItem (additive).

Changes

Engine core (3 engines × ValidateOutput)

  • Go operator.go: remove hasCW check from Recall branch
  • Java OperatorType.java: remove hasCommonWrites check from RECALL case
  • C++ engine.cpp: remove has_cw check from "recall" branch

recall_static enhancement (enables fixture + fuzz testing)

  • New optional set_common param (type any, default null): JSON object of common fields the recall writes
  • Three engines + codegen synchronized; cross-validate section 1 green

Differential fuzz enhancement

  • ~30% of generated recall_static ops now write random common fields
  • Fields flow into prev_common_outputs for downstream consumption
  • 250-round go+java smoke: 0 divergences

Tests

  • Go: new validate_output_test.go (6 cases: recall may write common, still forbids item mutations, transform/observe contracts, empty-output clean); scheduler E2E TestRunRecallWritesCommonConsumedDownstream
  • Java: new ValidateOutputTest.java (6 mirrored cases)
  • C++: test_engine.cpp reversed from "Recall must not SetCommon" error assertion to positive "Recall may SetCommon (downstream consumable)" + new "Recall must not SetItem" negative case

Documentation

  • dag-engine.md: type constraint matrix updated
  • operator-contract.md: type table + SetItemColumnFloat64 note corrected

Test plan

  • Go full test suite (19 packages)
  • Java full test suite (269 tests)
  • C++ test_engine (validate_output cases pass)
  • cross-validate section 1 (codegen schema parity, byte-level match)
  • differential-fuzz 250 rounds go+java, seed=7, 0 divergences
  • CI exercises all migrated paths end-to-end

Liam0205 added 2 commits July 15, 2026 23:58
Remove the SetCommon restriction from Recall's ValidateOutput in all
three engines — Go, Java, C++. A Recall may now emit request-level
common state (e.g. a recall-generated request id) that downstream
operators consume. The DAG dependency layer already builds correct
RAW/WAW/WAR edges from CommonOutput regardless of operator type,
confirmed by a three-case probe before this change.

Recall's common write is a normal mutating hazard participant:
- Two recalls writing the same common field → WAW serialized.
- Two recalls writing different common fields → item additive
  parallelism preserved.
- Downstream common reader → RAW edge from the recall (automatic).

Still forbidden: SetItem, RemoveItem, SetItemOrder (only AddItem for
items). The restriction half that guards data integrity is unchanged.

recall_static gains an optional `set_common` param across all three
engines + codegen, enabling fixture-driven testing and diff-fuzz
coverage of the new path.

differential-fuzz.py: ~30% of generated recall_static ops now write
random common fields, injecting them into prev_common_outputs for
downstream consumption. 250-round go+java smoke verified 0 divergences.

Tests:
- Go: ValidateOutput direct tests (new file), scheduler E2E
  (recall writes common → downstream transform reads it)
- Java: ValidateOutputTest (6 cases mirroring Go)
- C++: test_engine.cpp reversed to "Recall may SetCommon" positive
  test + new "Recall must not SetItem" negative test
- cross-validate section 1 (codegen schema parity) green
- architecture/dag-engine.md: Recall row in the type matrix now lists
  AddItem + SetCommon with "可选写 common" role description.
- reference/operator-contract.md: same table updated; the
  SetItemColumnFloat64 note corrected from "Recall 违规" to the
  accurate list of types that disallow item writes.
@github-actions

Copy link
Copy Markdown
Contributor

🔍 PR 审查

项目 结果
结论 ✅ APPROVE
审查截止 f2a544d6a2e024b346ceed309edfe59e52fdd106

代码良好。三个引擎(Go/Java/C++)对称移除 Recall 的 SetCommon 限制,配套测试、codegen、fuzz、文档同步齐全,仅发现 1 个跨引擎错误信息一致性的小问题。

🟢 小问题 (1)

  • 文件: pine-cpp/operators/recall/recall_static.cpp 代码链接
  • 问题: set_common 非法类型时,C++ 直接调用 it->second.as_object(),若传入非 object 会抛出通用错误 ConfigError("JSON value is not object")(见 pine-cpp/src/config/json.cpp:82)。而 Go(pine-go/operators/recall/static.go)和 Java(pine-java/.../RecallStatic.java)都给出了明确的 recall_static: 'set_common' must be a JSON object, got ...
  • 建议: 本项目强调跨引擎错误信息字节一致,可考虑在 C++ 侧先判 is_object() 再给出同款专用错误信息。属于健壮性优化,非阻塞——set_common 为测试/fuzz 用途参数,正常路径不会触发。

其他检查点(均通过)

  • Go ValidateOutput 的 Recall 分支正确保留 SetItem/RemoveItem/SetItemOrder 校验,hasCW 变量仍被 Filter/Merge/Reorder/Observe 分支使用,无死代码。
  • C++ has_cw、Java hasCommonWrites 同理仍被其他分支使用。
  • fuzz 中 FIELD_POOL(common)与 ITEM_FIELD_POOL(item)字段不相交,set_common 写入的字段经 common_out → new_common_out → prev_common_outputs 正确流向下游算子。
  • 测试覆盖完整:Go/Java 新增 ValidateOutput 直接单测 + Go 调度器 E2E(recall 写 common → 下游 transform 读取),C++ 从「Recall 禁止 SetCommon」反转为正向用例 + 新增「Recall 禁止 SetItem」负向用例。
  • 文档(dag-engine.mdoperator-contract.md)类型约束矩阵同步更新,SetItemColumnFloat64 备注修正准确。

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

🔍 PR 增量审查

项目 结果
结论 ✅ APPROVE
审查范围 f2a544d..273c966

增量提交 273c966 正好修复了上轮审查提出的 C++ set_common 类型校验缺失问题:在 as_object() 迭代前加了 is_object() 前置判断,非法类型时抛出专用错误而非通用的 ConfigError("JSON value is not object")。改动正确,逻辑与错误路径都无问题。

🟢 小问题 (1)

  • 文件: pine-cpp/operators/recall/recall_static.cpp 代码链接
  • 问题: C++ 错误信息为 recall_static: 'set_common' must be a JSON object,缺少 Go/Java 都带的 , got <type> 后缀:
    • Go (pine-go/operators/recall/static.go:70): ... must be a JSON object, got %T
    • Java (pine-java/.../RecallStatic.java:49): ... must be a JSON object, got " + GoTypeNames.of(sc)
  • 建议: 本项目强调跨引擎错误信息字节一致,且 _helpers.hpp 已提供 Go-reflect 风格的 json_type_name(const Variant&)(sibling recall_resource.cpp 已在用),可补齐后缀实现完全对齐:"recall_static: 'set_common' must be a JSON object, got " + operators::json_type_name(it->second)。非阻塞——set_common 的非法类型路径为测试/fuzz 用途,不在 fixtures/cross-validate 覆盖内,正常路径不会触发。

其他检查点(均通过)

  • RegistryError(op_name_, msg) 使用 2-arg 形式,产出 pine: registry error [recall_static]: 前缀,与 pine-go RegistryError 格式约定一致。
  • 前置 is_null() 判断保留,set_common 为 null 时仍走「不写 common」路径,行为不变。
  • 上轮审查其余检查点(三引擎对称移除 SetCommon 限制、hasCW/has_cw/hasCommonWrites 无死代码、fuzz 字段流向、测试与文档同步)本次增量未触及,维持通过。

🤖 Generated with Claude Code

@Liam0205
Liam0205 force-pushed the feat/recall-set-common branch from 273c966 to 9aa31dc Compare July 15, 2026 16:20
@github-actions

Copy link
Copy Markdown
Contributor

🔍 PR 增量审查

项目 结果
结论 ✅ APPROVE
审查截止 9aa31dc05f7a58f4d8210902eacd5f62c2e5de75

增量提交 9aa31dc 正好补齐了上轮审查提出的最后一个小问题:C++ set_common 非法类型的错误信息缺少 , got <type> 后缀。现已完全对齐三引擎。

✅ 上轮小问题已修复

  • 文件: pine-cpp/operators/recall/recall_static.cpp 代码链接
  • 改动: 在 is_object() 前置判断的错误信息里追加了 + pine::operators::json_type_name(it->second),得到 recall_static: 'set_common' must be a JSON object, got <type>
  • 验证跨引擎字节一致:
    • Go (pine-go/operators/recall/static.go:70): ... got %T(如 map[string]interface {}stringfloat64
    • Java (pine-java/.../RecallStatic.java:49): ... got " + GoTypeNames.of(sc)
    • C++ json_type_name (pine-cpp/operators/_helpers.cpp:33-53) 返回 map[string]interface {} / string / float64 / []interface {} / bool / <nil>,与 Go %T 输出一致 → 三引擎错误信息现完全对齐。

其他检查点(均通过)

  • _helpers.hpp 已在 recall_static.cpp:5 引入,json_type_name 声明可见,编译无误;sibling recall_resource.cpp 已在用同一函数,风格一致。
  • RegistryError(op_name_, msg) 沿用 2-arg 形式,产出 pine: registry error [recall_static]: 前缀,与 pine-go 约定一致。
  • 前置 is_null() 判断保留,set_common 为 null 时仍走「不写 common」路径,行为不变。
  • 上轮审查其余检查点(三引擎对称移除 SetCommon 限制、hasCW/has_cw/hasCommonWrites 无死代码、fuzz 字段流向 common_out → prev_common_outputs、Go/Java/C++ 测试与文档同步)本次增量未触及,维持通过。

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

🔍 PR 增量审查

项目 结果
结论 ✅ APPROVE
审查范围 9aa31dc..9c640d4

增量提交 9c640d4 为纯版本号提升 0.10.110.10.12,覆盖 67 个文件(apple/_version.pypine-go/version.gopine-cpp/include/pine/pine.hpppine-java/pom.xml、以及各 fixtures/testdata 里的 _PINEAPPLE_VERSION 字段)。逐行核对:所有改动均为版本字符串替换,无任何逻辑变更(过滤版本行后无剩余 diff)。

功能主体(三引擎移除 Recall 的 SetCommon 限制、recall_static 新增 set_common 参数、fuzz 增强、测试与文档同步)已在前两轮增量审查(f2a544d..273c966、截止 9aa31dc)中审查通过,本次增量未触及,维持通过。

其他检查点(均通过)

  • 版本号三引擎 + 打包元数据(pine-java/pom.xmlpine-cpp/include/pine/pine.hpppine-go/version.goapple/_version.py)同步一致,无遗漏。
  • fixtures/testdata 的 _PINEAPPLE_VERSION 与代码版本对齐,避免 cross-validate schema 版本校验失配。
  • bump 提交独立,未夹带其他改动,符合发版惯例。

🤖 Generated with Claude Code

@Liam0205
Liam0205 merged commit c353c04 into master Jul 16, 2026
21 checks passed
@Liam0205
Liam0205 deleted the feat/recall-set-common branch July 16, 2026 00:22
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.

1 participant