[infra] Add an api/ contract review guide

[weiqingy]

Linked issue: #894

Purpose of change

Continues item 2 of #894 by filling the api/ contract row that #911 left as planned. This is the third guide, after the merged runtime-state-recovery.md and python-java-bridge.md.

review-guides/api-contract.md follows the same shape as both: a focused checklist that narrows the full passes in code_review.md, and two examples drawn from real review threads on merged PRs, each linking the specific comment.

The six bullets are ordered by how often a reviewer will hit one, and each names a failure that is silent in this repo rather than one the build catches. A new integration whose YAML alias was never added still resolves, because both loaders pass an unrecognized name through unchanged. A config option added on the Java side only passes every fast CI job, since the bidirectional parity check runs solely in the cross-language lane. A doc that contradicts the code ships green, because nothing in pull request CI builds the doc site.

One bullet is worth calling out because it removes work rather than adding it. The row's Focus text names deprecation, but there is no deprecation mechanism in this repo, so the question a reviewer should be asking is whether a removal is complete, not whether something should be deprecated first. I left the Focus cell alone and addressed it in the guide instead.

It carries a Validation block, like python-java-bridge.md and unlike the merged runtime-state-recovery.md. Two of its five commands are the reason it is there: ConfigOption parity and ResourceName parity are standalone scripts rather than tests, so neither mvn test -pl api nor pytest flink_agents/api reaches them, and today they run only behind the Docker-backed e2e lane. Both finish in well under a second when invoked directly.

That block is why this guide is 80 lines against the merged siblings' 31 and 55. The checklist itself is the same six bullets; the difference is five commands instead of two.

The last commit also touches python-java-bridge.md, which is already merged. Both guides published a Validation block whose Java command resolves flink.version from the root pom.xml while its Python command named a Flink version some other way, so the two lanes in one block could run different Flinks. Both Python lanes now read the property at run time instead. Fixing only this guide would leave two files in one directory giving contradictory guidance on the same question.

The remaining two rows, dist and docs-only, stay planned and will follow the same way.

Tests

Not applicable, documentation only, no logic.

./tools/check-license.sh passes. No license header is needed, since tools/.rat-excludes already covers review-guides/ and .licenserc.yaml ignores **/*.md.

Every checklist claim was checked against source rather than written from memory, and several drafts were discarded when the source disagreed. The claim about cross-language snapshots was verified by experiment rather than by reading: adding a field to a built-in event on both sides and regenerating only the Java snapshots fails exactly one test, the Python side's own stability test, while its cross-read counterpart still passes. Adding the field on the Java side only and regenerating fails nothing at all, across 292 Java and 450 Python tests. Both of those outcomes are what the bullet now says.

All published commands were executed. Flags that made no difference were removed rather than left in for safety, including -Dspotless.skip=true, since spotless runs and passes and skipping it would hide a real CI failure. The two that remain were each confirmed load-bearing.

Both guides install PyFlink by reading flink.version out of the root pom.xml rather than naming a version, so each block's two lanes share one source of truth. The derivation returns 2.3.0, and dependency:tree on api and on runtime puts the Flink dependencies that carry flink.version at that same 2.3.0, so both lanes demonstrably run the same Flink. pytest flink_agents/api flink_agents/plan reports 450 passed and 11 skipped; the bridge guide's wider selection, pytest flink_agents/runtime flink_agents/api flink_agents/plan, reports 614 passed and 11 skipped; mvn --batch-mode test -pl runtime -am is BUILD SUCCESS across 919 tests. The read was also exercised on a cold local Maven repository, where it still prints only the version, and against a copy of the POM carrying a different value, which the command follows.

Both review permalinks were re-fetched and both cited PRs re-confirmed merged. The relative link was checked by hand, since no workflow in this repo validates markdown or checks links.

API

No. No code or public API change.

Documentation

• doc-needed
• doc-not-needed
• doc-included

Was this patch authored or co-authored using generative AI tooling?

• Yes
• No

Generated-by: Claude Code 2.1.228 (Claude Opus 5)

[wenjin272]

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.

[weiqingy]

Good point, and it turns out not to be hypothetical. BaseChatModelConnection.chat(List, List, Map, Object) is that four-argument default, and it throws rather than forwarding a schema it cannot translate.

The bullet now reads:

Compiling is not the same as honoring the new argument: a default that forwards to the older signature drops it in silence, so check that the default rejects what it cannot honor, and that an override gating it behind a capability probe leaves a fallback in force or fails, rather than silently doing neither.

I could not find a way to express the caller half here. supportsNativeStructuredOutput is protected, and nothing calls the four-argument overload in production, so I aimed the clause at the overrides, where the probe actually gets consulted. Not a defect report, since no schema reaches those branches today. Does that match what you had in mind, or would you rather it name the caller case and point at #912?

On the wording, I made it a disjunction rather than a flat "must fail", since BaseChatModelConnection.java:59-61 asks an unrecognized model to report false and degrade to the fallback. Is that the boundary you were drawing?

[wenjin272]

This feels a little too absolute. The repository currently has no formal deprecation process, but under the beta policy the key question is whether there is a concrete compatibility obligation. Could we instead say that reviewers should prefer complete removal when no such obligation exists?

[weiqingy]

Agreed, and it was contradicting this guide's own #756 row, which frames the same question as a judgment. Reworded to:

There is no deprecation mechanism in this repo, so an API is either kept or deleted outright. Under the beta policy, prefer deleting unless a concrete compatibility obligation requires keeping it.

I kept the missing-mechanism sentence since that is what rules out a third option, and moved the preference onto the beta policy, where code_review.md already puts it. Does that split read the way you meant?

[wenjin272]

Could we avoid installing an unpinned latest apache-flink here? This validation command may become unstable when a newer unsupported version is released. It would be safer to use a repository-supported version or derive one from the test matrix/default.

[weiqingy]

Pinned to apache-flink~=2.3.0.

I anchored it to the root pom.xml's flink.version rather than tools/ut.sh, and would welcome a second opinion. The Java command directly above resolves 2.3.0 through ${flink.version}, so ut.sh's 2.2 would have split the two lanes. tools/ut.sh also seems to disagree with itself: :24 sets 2.2, while the help text at :48 says it runs all versions. Worth a separate issue, or have I misread it?

The pinned lane resolves 2.3.0 and reports the same 450 passed and 11 skipped as 2.2.1, so it bounds the install without narrowing what is known to work.

One limit: nothing greps review-guides/ for version strings, so this will not follow a future bump on its own. If you would rather derive it the way python-java-bridge.md does, without a number, that is an easy trade.

[wenjin272]

I would prefer deriving the version from the root POM rather than hard-coding 2.3.0. The Java validation command already uses its flink.version, so sharing that source of truth keeps both lanes aligned and prevents the guide from becoming stale after a future version bump.

You also read tools/ut.sh correctly. It defaults to Flink 2.2 when -f is omitted, despite claiming to run all versions. Additionally, its non-E2E Java path ignores flink_versions and uses the root POM’s Flink 2.3.0, while the default Python path installs Flink 2.2.x. Multi-version coverage comes from the CI integration-test matrices explicitly passing -e -f, not from the script’s default behavior.

This is worth tracking separately and further supports using the root POM, rather than tools/ut.sh, as the version source here.

[weiqingy]

Done, in both guides:

uv pip install "apache-flink==$(mvn -q -N --batch-mode -f ../pom.xml help:evaluate -Dexpression=flink.version -DforceStdout)"

== rather than ~=, so the two lanes land on the same three-component version rather than only the same minor line. dependency:tree puts the Flink dependencies carrying flink.version at 2.3.0 on both api and runtime, and pointed at a copy of the root POM with a different value the command follows it.

I also applied it to python-java-bridge.md, which is already merged, since its Python bullet had the same split and two guides in one directory answering this differently seemed worse than the extra file. That does widen the PR into merged code though. Would you rather I split it out and send it separately?

One cost worth flagging: that guide's parenthetical was the only pointer either guide had to the supported version set, and neither names it now. Worth restoring somewhere, or is one derived version per block the right scope here?

I will open the tools/ut.sh issue and take a first pass at it.

[wenjin272]

Thanks for addressing my comments. LGTM

[weiqingy]

@wenjin272 Thanks for the review. #1007 was created to address the tools/ut.sh issue discussed above.