weiqingy commented on PR #965: URL: https://github.com/apache/flink-agents/pull/965#issuecomment-5276607032
> Thanks @weiqingy for putting this together and for trying the Implementation Description experiment across a larger, two-language change. Overall, the description was useful: it let me establish the runtime flow and intended contracts without first reconstructing them from the diff. I think it could be made more review-oriented in four ways: > > 1. **State the purpose more directly.** The current section mainly explains why the previous implementation rejected the schema, but the intended user-visible outcome is less immediate. A shorter version could be: > > This PR makes the existing provider-independent `output_schema` API work with Anthropic in both Java and Python. Supported schemas are translated into Anthropic's native `output_config`; unsupported combinations continue through the existing prompt-based fallback. The goal is consistent structured-output behavior across providers and across the Java and Python integrations, without introducing a new public API. > 2. **Add a small interaction/decision table.** The numbered contracts describe individual paths well, but make cross-feature combinations harder to inspect. A table covering model capability, schema type, caller `output_config`, tools, and JSON prefill would make conflicts more visible. The `output_config` plus prefill case discussed above is a good example of something such a table could surface. > 3. **Separate behavioral contracts from implementation invariants.** User-observable behavior should remain in the main contract section, while details such as the predicate reading no instance state, 3-arg forwarding, and identical model-list ordering could move to implementation notes. > 4. **Keep the test-to-contract mapping, but move the detailed test names and mutation notes into a `<details>` block or follow-up comment.** The main body could summarize coverage by risk and explicitly list what was not verified. This would reduce the reading cost without losing the evidence useful for Stage 2. > > My takeaway is that the main description should prioritize the information needed for human review: the purpose, interaction decisions, behavioral contracts, risks, and coverage. Detailed implementation invariants and test mappings can remain available through a `<details>` block or a follow-up comment without competing for the reviewer's attention, allowing us to keep the main description as concise as possible. Thanks for the feedback. I have added them to my notes for the experiment. The description on this PR was auto-generated by the agent from an earlier version of the template. Once the template improves, the descriptions should come out more readable for a human reviewer. -- 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]
