jdaugherty commented on PR #16599: URL: https://github.com/apache/grails-core/pull/16599#issuecomment-6150773517
Thanks for the thorough review. Everything is addressed in f77d9182d9, by finding: **grails-async streaming** 1. Every stream now ends with the `complete` event, whatever ended it; its data says why: empty, `timeout` or `error`. A client that closes on `complete`, as the async chapter tells it to, never reconnects after a failure. `PublisherActionResultTransformerSpec` and the example app's `StreamingFunctionalSpec` cover the failed stream. 2. There is no event named `error` any more. A failure is the `complete` event with `error` as its data, so nothing collides with `EventSource`'s own event; the docs show how to tell the two endings apart. 3. Every write to the response, and the `done` check before it, now happens under one lock in `ServerSentEventSubscriber`. A new test blocks the write of an item on the publisher thread, reports the timeout from another thread, and checks the timeout waited and the events follow one another on the wire. 4. `Mono` stays in: the transformer streams any `Publisher`, and the Javadoc, the streaming section, the what's new note and the agent skill now say so explicitly, including that such an action no longer renders its view and that `grails.async.streaming.enabled: false` restores it. A `Mono` case was added to the transformer spec and to the example app. 5. Nothing of a failure reaches the client now; the type stays in the server log. The spec asserts the exception's type and message are absent from the response. **grails-testing-support-ai** 6. `AiTestingVectorStoreAutoConfiguration` is now guarded by `spring.ai.vectorstore.type=none`, so a store the application selects, such as PgVector, is no longer pre-empted; `AiTestingAutoConfigurationSpec` has the `pgvector` case. The bean is a `ResettableSimpleVectorStore`, so specs sharing a context can `clear()` it; the Javadoc explains the evaluation-order reason for the guard. 7. `AiStubEnvironmentPostProcessor` defaults every selection property Spring AI 2.0.1 reads to `none`: chat, embedding (and its `text` and `multimodal` variants), image, audio transcription, audio speech, moderation and the vector store. The spec pins the list against `SpringAIModelProperties`. 8. `AiUnitTest` now registers each stub only when the context holds no bean of its type, so a chat model, embedding model, `ChatClient.Builder` or `VectorStore` the spec defines wins and is never silently replaced; the getters look beans up by type, and reset covers every `ScriptedChatModel` in the context. `AiUnitTestOwnBeansSpec` covers a spec-defined model. The registration point itself is the one every Grails testing trait uses (`ControllerUnitTest`, `DataTest` and the others register their beans through `defineBeans` in a `setupSpec` interceptor as well), so a bean from `doWithSpring()` that needs the builder at creation should take it from the context lazily, or be defined in `setupSpec`; the trait's Javadoc and the testing page now say so. 9. `stream()` is wrapped in `Flux.defer`, and recording the prompt and taking the response happen in one synchronised step. Tests cover the unsubscribed stream, a `retry()`, and 200 concurrent calls each getting a scripted response once. 10. Echoing is intended, so a test that does not care about the answer needs no script, but you are right that it can hide a short script: `chatModel.strict = true` makes the model fail with the prompt number and text when the script runs out. Documented in the testing page and the skill. 11. The unit-test stub now binds `grails.ai.stub.embeddingDimensions` from the environment with Boot's `Binder`, so `doWithConfig` sizes it as the auto-configuration does; covered in `AiUnitTestOwnBeansSpec`. **Forge** 12. `SpringAiProviderFeature` is now a `OneOfFeature`, so selecting two providers fails with the usual "There can only be one of the following features selected" message; `GrailsSpringAiSpec` has the two-provider case, and the installation page says the provider features exclude one another. **grails-spring-ai and docs** 13. In this repository `org.apache.grails.testing.*` is the public namespace of the newer testing modules rather than an internal one: the guide already tells users to import `org.apache.grails.testing.http.client.HttpClientSupport`, `org.apache.grails.testing.cleanup.core.DatabaseCleanup` and `org.apache.grails.testing.mongo.AutoStartedMongoSpec`, and `grails.testing.ai.AiUnitTest` sits next to `grails.testing.services.ServiceUnitTest` for the same reason those do. So I left `ScriptedChatModel` where it is; if the project would rather have one package for the whole module, that is a quick move. 14. Good catch, and confirmed against 2.0.1: `ChatClientAutoConfiguration` takes the registry with `getIfUnique(() -> ObservationRegistry.NOOP)`, and the prompt and completion handlers are `ObservationHandler` beans that only Boot's `ObservationAutoConfiguration` attaches to a registry. A web application happened to have one through `grails-dependencies-starter-web` → `spring-boot-starter-micrometer-metrics`, but the plugin now brings `spring-boot-micrometer-observation` itself so the development defaults work on any classpath. `SpringAiPromptLoggingSpec` drives a chat client through the real auto-configurations and asserts the prompt and the completion are logged, and that nothing is logged with the settings off. The configuration page notes the registry. 15. Renamed to `SpringAiAutoConfigurationSpec`. -- 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]
