jdaugherty commented on PR #16599:
URL: https://github.com/apache/grails-core/pull/16599#issuecomment-6156132046
Thanks, this was a very useful second pass. Everything is addressed in
ea0698edeb, by finding:
**Streaming**
1. I kept streaming for multi-valued publishers whatever the request
accepts, and followed Spring MVC for single-valued ones. A Grails action does
not negotiate the body format on its return value: a `Map` is a model and
`render`/`respond` ignore `Accept` too, negotiation happens inside the action
with `withFormat`, and the RxJava plugin's `Observable` support streamed on the
same terms. An action that returns a `Flux` is a streaming endpoint by
declaration, so that stays. A `Mono`, and every other single-valued type
Spring's `ReactiveAdapterRegistry` knows, is now resolved like a `Promise`: a
`Map` is the model of the view, anything else leaves the response to what the
action rendered, a failure fails the request, and the response is not
committed. Covered in `PublisherActionResultTransformerSpec` and by the example
app, where a `Mono` that renders JSON gives the JSON response. If you would
still rather have `Accept` negotiation for `Flux`, that is a contained change
on top of th
is.
2. A stream now has no timeout unless `grails.async.streaming.timeout` sets
one, exactly as Spring MVC's SSE emitters (`STREAMING_TIMEOUT_VALUE = -1`), and
neither `spring.mvc.async.request-timeout` nor the container default applies to
it. The metadata, both guide sections and the skill say so; the test checks the
async context gets `-1`.
3. The two built-in renderers are now always consulted last, structurally,
in `ControllersAsyncGrailsPlugin`, so a renderer wins over them whether or not
it is `Ordered`; the SPI Javadoc and the guide say that instead of talking
about precedence values. A test registers a plain closure renderer after the
plugin's beans and sees it win for its type.
4. There is now `upgrading81x` in the upgrade guide, naming
`grails.async.streaming.enabled: false` and the `Mono` rule, and what's new
links to it. The comma is gone.
5. All applied: import order and the blank line, the two test names, and
`volatile` on the flag.
**Testing support**
6. `reset()` turns `strict` off, so it is per feature (or per `setup()`) and
never leaks into a later feature or, in integration tests, a later spec.
Documented, and `ScriptedChatModelSpec` covers it.
7. The stubs now stay off outside the test environment unless
`grails.ai.stub.enabled` is set to `true` explicitly, and the post-processor
logs a WARN naming the module and the property when it finds itself elsewhere.
The environment is recognised from `grails.env`, which the Grails Gradle plugin
sets for every test task, or from the `test` Spring profile Grails adds for its
environment. `AiStubEnvironmentPostProcessorSpec` covers production,
development, the profile, neither and the explicit opt-in.
8. You were right: `GrailsApplicationPostProcessor` is neither
`PriorityOrdered` nor `Ordered`, so `resources.groovy`, `doWithSpring` and
`beanRegistrar` beans land after `ConfigurationClassPostProcessor` evaluated
`@ConditionalOnMissingBean`. The stubs are now `@Fallback` beans instead of
`@Primary`, so Spring passes them over whenever another candidate exists: the
application's model is injected and the stub is left alone, with no ambiguity
error. Two proofs: `AiTestingAutoConfigurationSpec` registers the application's
beans from a post-processor with the same ordering as the Grails one, and the
example app has `OwnChatModelFunctionalSpec`, booted with an application class
whose `doWithSpring` defines a `ChatModel`, which asserts the application's
model answers `/chat/ask` while the stub bean is a fallback and receives
nothing. The testing page describes the fallback rule instead of the "backs
off" wording.
9. `AiTestingVectorStoreAutoConfiguration` is now
`@AutoConfigureOrder(LOWEST_PRECEDENCE)`, so it sorts after a provider
auto-configuration that declares no order; the spec has a provider-style
embedding auto-configuration selected with `spring.ai.model.embedding=provider`
and checks the stub store is built over that model.
10. Fixed: the fallback to the fixed unit vector now applies whenever the
norm is zero, words that cancel out included. The test uses one dimension,
where every word lands on the same component, and checks every pair of forty
words is a unit vector, which the old code could not satisfy.
11. All three: the concurrency test now uses responders that record the
prompt they answer and asserts `prompts[i]` is the prompt responder `i` saw;
the production code takes the selection keys from `SpringAIModelProperties`,
and the spec reflects over that class so a Spring AI release that adds a model
kind fails the build; `AiUnitTestOwnBeansSpec` now defines a chat model,
embedding model, `ChatClient.Builder` and vector store of its own and checks
none of the four stubs is registered, while `AiUnitTestConfiguredSpec` keeps
the partial case and the dimensions setting.
**Forge, docs and skill**
12. Both now read `grails -t forge create-app …`.
13. Registered in `external-references.yml`, both end-to-end skill builds,
`PUBLISHED_SKILLS` in `AgentSkillsSpec` and the README table.
14. The provider features now support only `WEB` and `REST_API`;
`grails-spring-ai` itself still supports plugins. The plugin command references
no longer list the provider features, and the spec checks the application types.
15. Confirmed against the jars: `OpenAiSetup` and `AnthropicSetup` in 2.0.1
read `OPENAI_API_KEY` and `ANTHROPIC_API_KEY` from the environment when no key
is configured. The features no longer write `spring.ai.<provider>.api-key`, the
generated YAML is asserted to contain no key, and the installation page and the
skill now say to leave the key to the environment and never to use a
placeholder, for the reason you give.
16. All four: `8.1.0-SNAPSHOT > *`; what's new states "Spring AI
{springAiVersion}"; the AI section is "Streaming Model Responses"; the
`SbomPlugin` comment gives the `http://` URLs the POMs declare.
On the CI failure on the previous head: the Java 25 functional job lost
Maven Central mid-run ("Repository MavenRepo is disabled due to earlier error",
`spring-ai-bom:2.0.1` and `jaxb-runtime` unresolvable), so it is a rerun, not a
code problem.
--
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]
