doc: adaption#88
Conversation
There was a problem hiding this comment.
The PR contains one concrete correctness issue worth addressing: the cds-feature-ai-core/README.md programmatic API example imports RptModelSpec from cds-feature-recommendations, a separate module not bundled with cds-feature-ai-core alone — readers following the example without that dependency on the classpath will get a compile error. The other changes are clear documentation improvements with no other substantive issues.
PR Bot Information
Version: 1.26.0
- Event Trigger:
pull_request.opened - LLM:
anthropic--claude-4.6-sonnet - File Content Strategy: Full file content
- Correlation ID:
b52a3f81-a40d-4c47-84c7-8ebaa3f13837
| import com.sap.cds.feature.aicore.api.ResourceGroupContext; | ||
| import com.sap.cds.feature.aicore.generated.cds4j.aicore.AICore_; | ||
| import com.sap.cds.services.cds.RemoteService; | ||
| import com.sap.cds.feature.recommendation.api.RptModelSpec; |
There was a problem hiding this comment.
Bug: Wrong import package for RptModelSpec — the actual source file lives under com.sap.cds.feature.recommendation.api (as confirmed by the repo tree), but the import shown here uses com.sap.cds.feature.recommendation.api — actually the diff shows com.sap.cds.feature.recommendation.api.RptModelSpec yet the package directory is com/sap/cds/feature/recommendation/api/RptModelSpec.java. The import on line 79 uses com.sap.cds.feature.recommendation.api.RptModelSpec which is correct, but the introductory sentence on line 71 says the plugin exposes "three event contexts" — ResourceGroupContext, DeploymentIdContext, and InferenceClientContext — yet RptModelSpec (from a different module, cds-feature-recommendations) is imported without any note that it requires adding cds-feature-recommendations as a dependency. A reader using only cds-feature-ai-core will get a compile error. Add a note that RptModelSpec comes from cds-feature-recommendations, or replace it with the lower-level ModelDeploymentSpec which belongs to cds-feature-ai-core.
| import com.sap.cds.feature.recommendation.api.RptModelSpec; | |
| import com.sap.cds.feature.aicore.api.ModelDeploymentSpec; // from cds-feature-ai-core | |
| // import com.sap.cds.feature.recommendation.api.RptModelSpec; // only if cds-feature-recommendations is on the classpath | |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| | Profile | Scope | Effect | | ||
| |---------|-------|--------| | ||
| | _(default)_ | Root | Builds all modules; runs spring integration tests | | ||
| | `with-integration-tests` | Root | **Active by default**; includes `integration-tests/` and `coverage-report/`. Deactivate with `-P-with-integration-tests`. | |
There was a problem hiding this comment.
Bug: The "Deactivate with" hint in the table cell is misleading — -P-with-integration-tests deactivates the profile, but the text reads "Deactivate with -P-with-integration-tests", which correctly describes deactivation syntax yet contradicts the column header "Effect". More critically, the profile name with-integration-tests uses a leading - for deactivation, but writers might accidentally copy the string literally into a Maven command. This is fine as written; however the removed skip-integration-tests row is referenced nowhere else — confirm skip-integration-tests is truly gone from the root pom.xml so readers don't try to use it.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
Documentation Improvements and Clarifications
Documentation
📝 Improved and corrected documentation across multiple README files, clarifying prerequisites, AI Core binding setup, the
AICoreCDS service internals, the programmatic API, and integration test profiles.Changes
README.md: Added Maven 3.6.3+ as a prerequisite, removed the outdated Node.js global install requirement, clarified the hermetic build note, updated the integration-test command frommvn test -pl integration-tests/spring -amtomvn verify, and added a reference to the integration-tests README.cds-feature-ai-core/README.md: Clarified that theAICoreCDS service is internal (@protocol: 'none') and not exposed via OData. Updated the AI Core binding section to replace bullet-list binding methods with a focused CAP CLI example. Expanded the entity table to reflect UPDATE operations and thestopbound action. Replaced the oldAICoreServiceprogrammatic usage section with a fully working step-by-stepRemoteService-based code snippet. Added a new Public API table documentingResourceGroupContext,DeploymentIdContext,InferenceClientContext, andModelDeploymentSpec. Removed the outdated## Programmatic Usagesection that referenced the removedAICoreServicefacade.cds-feature-recommendations/README.md: Replaced the single model-deduplication note with a two-option guide (Option A: Java-internal only; Option B: OData exposure). Clarified the@cds.odata.valuelistannotation derivation. Fixed a configuration YAML key fromrequires:toai:. Added supported CDS HANA types to the field type table. Corrected the mock client name fromMockAIClienttoMockRecommendationClient. Minor whitespace/trailing-space fixes.integration-tests/README.md: Updated the "skip integration tests" command from the non-existentskip-integration-testsprofile to the correct-P-with-integration-testsdeactivation syntax. Updated the profiles table accordingly. Loweredcds-feature-recommendationscoverage thresholds to 0% with an explanatory note about tightening them once APIs stabilise.PR Bot Information
Version:
1.26.0pull_request.openedb52a3f81-a40d-4c47-84c7-8ebaa3f13837anthropic--claude-4.6-sonnet