hanzhe-one commented on PR #7937: URL: https://github.com/apache/shenyu/pull/7937#issuecomment-6311428588
Thanks for the review. Three of the four points are addressed in 916751f: - **`Constants.RESPONSE`** — removed from the shared interface. The field is only meaningful to this observer, so it is now a private `RESPONSES_EVENT_OBJECT` in `AiTokenLimiterPlugin`, and `shenyu-common` is left with just `OUTPUT_TOKENS`, which mirrors the existing `COMPLETION_TOKENS`. - **Implicit Chat invariant** — made explicit at the fallback site: "Chat never uses a top-level \`response\` field, so the fallback cannot shadow a Chat payload." - **Split-buffer test comment** — reworded as suggested: neither buffer holds a complete event, so the streaming JSON-parse path finds nothing and the regex fallback over the accumulated SSE tail accounts the usage. On the third point (`AiTokenLimiterResponsesUsageTest` being coupled to the adapter's output format): I left it as is, because the premise does not hold for this test. It does not pin the adapter's output — it *feeds* the adapter and asserts on whatever bytes come back. `CHAT_COMPLETION` / `CHAT_STREAM` are the upstream-side Chat fixtures the test hands to the adapter; if the adapter changes its Responses output format, the observer receives the new format directly and the assertion still exercises the behavior we care about. There is therefore no stale fixture to debug when it fails: a failure means the limiter no longer accounts the adapter's current output, which is exactly the signal we want. Re-ran `mvn -pl shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-token-limiter -am test -Dtest='AiTokenLimiter*Test'`: 30 tests, 0 failures, checkstyle clean. -- 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]
