This is an automated email from the ASF dual-hosted git repository.
wenjin272 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/flink-agents.git
The following commit(s) were added to refs/heads/main by this push:
new 8d1702c9 [infra] Add change-type review guides with runtime/ template
(#911)
8d1702c9 is described below
commit 8d1702c96c1401b8f114831018ad2f8ada193a69
Author: Weiqing Yang <[email protected]>
AuthorDate: Sat Jul 25 00:28:45 2026 -0700
[infra] Add change-type review guides with runtime/ template (#911)
---
code_review.md | 15 +++++++++++++++
review-guides/runtime-state-recovery.md | 31 +++++++++++++++++++++++++++++++
tools/.rat-excludes | 3 ++-
3 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/code_review.md b/code_review.md
index b99d41d1..f78e7bd6 100644
--- a/code_review.md
+++ b/code_review.md
@@ -30,6 +30,21 @@ Keep these findings separate. A design-focused comment does
not need to prove an
immediate runtime bug, but it must explain the concrete complexity, ambiguity,
or maintenance risk introduced by the change.
+## Change-Type Review Guides
+
+For common change types, start from the focused guide below as an on-ramp, then
+apply the full passes in this document. Each guide narrows the checklist to the
+concerns that matter most for that area and links examples from past reviews.
+Guides load on demand, so the general passes here stay short.
+
+| Change type | Focus | Guide |
+|---|---|---|
+| `runtime/` state and recovery | serde and replay type fidelity, real
failure-path tests |
[review-guides/runtime-state-recovery.md](review-guides/runtime-state-recovery.md)
|
+| Python-Java bridge | cross-language parity, type mapping across Pemja |
planned |
+| `api/` contract | API shape, compatibility policy, deprecation | planned |
+| `dist` and dependency | shading, LICENSE and NOTICE, dist registration |
planned |
+| docs-only | facts match their source of truth | planned |
+
## Required Review Passes
1. Understand the issue or feature goal.
diff --git a/review-guides/runtime-state-recovery.md
b/review-guides/runtime-state-recovery.md
new file mode 100644
index 00000000..f03c5354
--- /dev/null
+++ b/review-guides/runtime-state-recovery.md
@@ -0,0 +1,31 @@
+# Review Guide: runtime/ State and Recovery
+
+Load this guide when a PR changes runtime state, checkpointing, serialization
of
+stored values, action-state handling, or async, mailbox, and recovery paths. It
+narrows the full passes in `code_review.md` to the ones that matter most for
this
+area; the general passes still apply.
+
+## Focused checklist
+
+- Trace every stored value through its full serialize, checkpoint, and restore
+ round-trip. Confirm the restored value keeps its original type instead of a
+ lossy or re-encoded form, such as a `byte[]` that returns as a base64
`String`.
+- Confirm a serde envelope or version tag scopes the value the way recovery
+ needs it, not only the way the happy-path write needs it.
+- Under the beta policy, question serde or state compatibility flags that only
+ exist to read old formats. Prefer removing them unless they carry a concrete
+ obligation.
+- For a bugfix, check the failure actually stops reaching the old path after
the
+ fix, not just in the one code path the PR touched.
+- Test the real runtime failure path through the operator, mailbox, or action
+ task, not a helper in isolation. Make failure-injection fixtures
deterministic
+ so the regression test exercises the bug on every run.
+- Cover async, retry, delayed-callback, and recovery paths when the change
+ affects them.
+
+## Examples from past reviews
+
+| Case | Pass it exercises | Review |
+|---|---|---|
+| A versioned serde envelope for stored `MemoryUpdate` values carried an
`isEnvelope` backward-compatibility flag. The review asked whether that
compatibility path was essential or should be dropped under the beta policy. |
Whether a serde compatibility channel must exist, or only keeps an old format
alive. |
[#874](https://github.com/apache/flink-agents/pull/874#discussion_r3541046130) |
+| A hotfix propagated a swallowed action-task `Error` out of the mailbox
instead of losing it in a `Future`. The review kept the linkage-error
regression fixture deterministic, since `System.nanoTime()` may return a value
that skips the injected failure, so the test exercises the real failure path on
every run. | Testing the actual runtime failure path with a deterministic
fixture. |
[#880](https://github.com/apache/flink-agents/pull/880#discussion_r3540383941) |
diff --git a/tools/.rat-excludes b/tools/.rat-excludes
index dcd7de72..94dacb5f 100644
--- a/tools/.rat-excludes
+++ b/tools/.rat-excludes
@@ -25,4 +25,5 @@ skills/*
.*\.yaml$
AGENTS.md
code_review.md
-CLAUDE.md
\ No newline at end of file
+CLAUDE.md
+review-guides/*
\ No newline at end of file
