wenjin272 commented on code in PR #961: URL: https://github.com/apache/flink-agents/pull/961#discussion_r3747310417
########## review-guides/api-contract.md: ########## @@ -0,0 +1,72 @@ +# Review Guide: api/ Contract + +Load this guide when a PR changes a public API surface: a signature or type in +`api/`, a new resource implementation, a config option, a YAML-visible name, or +anything a user's agent code calls. 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 + +- When a PR adds a public resource implementation, check that its short YAML + alias landed in both alias tables and the doc table. Nothing fails when all + three are skipped: each loader passes an unrecognized name through unchanged, + so the class stays reachable by fully-qualified name and no test notices the + omission. +- Regenerate the cross-language snapshots on both sides in the same change when + a field on a built-in event or on the agent plan is added, renamed, or + retyped. Each language pins its own serialization against its own committed + file, so refreshing one side leaves the other side's stability test failing. A + field added in only one language is caught by nothing, because the payload is + a free-form attribute map on the read side. +- Check that a new public config option landed on both languages' sides. A + Java-only option passes every fast CI job: the bidirectional parity check runs + only in the slow cross-language lane, and the in-tree guard is a hardcoded + count on the Python side. +- Treat the public base classes users extend as source-compatibility boundaries. + A new abstract method breaks every implementation, including ones outside this + repo, while a defaulted overload plus a capability probe does not. The Python Review Comment: Could we also call out semantic compatibility here? A defaulted overload can keep third-party implementations compiling while silently dropping a new argument. For example, a four-argument `chat` forwarding to the three-argument form can ignore `outputSchema`. Reviewers should also verify that the default fails explicitly, or that every caller enforces the capability probe and a safe fallback. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
