wenjin272 commented on PR #965:
URL: https://github.com/apache/flink-agents/pull/965#issuecomment-5265211746
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.
--
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]
