fix: draft-technical 422 on /predict, mocked-service crash; feat: opt-in recommendation mode#43
Open
sleibach wants to merge 3 commits into
Open
fix: draft-technical 422 on /predict, mocked-service crash; feat: opt-in recommendation mode#43sleibach wants to merge 3 commits into
sleibach wants to merge 3 commits into
Conversation
added 3 commits
July 21, 2026 01:53
The READ handler registered by `registerHandlersForRecommendations` was
attached to every non-db service, including external/mock services
(`kind: rest`, `kind: odata`, …). The handler then dereferences
`req.target.isDraft`, which throws TypeError for any free-form
`srv.send(new cds.Request({ method, path }))` call (no entity target
resolved), aborting the calling flow.
- cds-plugin.js: skip services where `srv.mocked === true` (set by
@sap/cds/lib/srv/cds-serve.js for external services without real
credentials). These services have no @UI.Recommendations model.
- lib/handlers/recommendations.js: defensive `if (!req.target) return next();`
as belt-and-suspenders for any other free-form READ that slips through.
Also adds optional chaining on the early-return clause for safety.
Repro before: a CAP project with an external service that falls back to
an in-process impl (e.g. `kind: rest` + `model: srv/external/Foo` and no
credentials in dev), making remote-style `srv.send` calls — crashes with
`Cannot read properties of undefined (reading 'isDraft')`.
The draft branch of the recommendations READ handler builds its column
list from all elements of req.target.actives, filtering out only audit
fields, LargeBinary and Vector. Association/composition elements pass
the filter — including the compiler-added DraftAdministrativeData on
draft-enabled entities.
Those elements have no RPT-1 dtype (cdsToPythonDtype returns undefined),
so they are omitted from data_schema but still present in the rows sent
to /predict. As soon as context rows resolve DraftAdministrativeData to
a non-scalar value (i.e. whenever other edit drafts of the entity
exist), AI Core rejects the whole payload with HTTP 422:
{"detail":[{"loc":["rows",7,"DraftAdministrativeData","str"],
"msg":"Input should be a valid string",...}]}
The error is swallowed (empty predictions returned), so recommendations
silently disappear while every draft READ with $expand=SAP_Recommendations
still pays a full failed round-trip to AI Core.
Fix: skip elements with isAssociation when building the column list.
Flattened foreign keys (e.g. author_ID) are separate scalar elements and
remain included — they are the actual prediction targets and features.
Adds a regression test that opens an edit draft (so an active context
row carries draft administrative data) and asserts the rows handed to
fetchPredictions contain no association keys or non-scalar values.
The plugin is opt-out today: every field with a value help auto-enrolls
for recommendations and each unwanted one must be silenced individually
with @UI.RecommendationState : 0. On large existing models this enrolls
dozens of fields at once and generates unnecessary /predict traffic.
Add a config flag to invert the default for controlled rollouts:
"AICore": { "recommendations": "opt-in" }
- "auto" (default): unchanged behavior — value-helped fields
auto-enroll, @UI.RecommendationState : 0 excludes.
- "opt-in": a field is enrolled only if it carries a truthy
@UI.RecommendationState (1 or a dynamic expression); everything else,
including value-helped fields, stays out.
@UI.RecommendationState : 0 wins in both modes, and entities with zero
enrolled fields get no SAP_Recommendations companion (existing guard).
Absence of the flag is byte-for-byte identical to current behavior.
Includes tests booting the bookshop in opt-in mode (own process under
node --test), README docs and a CHANGELOG entry.
SirSimon04
reviewed
Jul 21, 2026
SirSimon04
left a comment
Contributor
There was a problem hiding this comment.
Hi @sleibach, thank you for your contribution! In general, it looks really good and sounds like a good feature to us.
Could you remove those big comments or shorten them? Most of the times, the code speaks for itself
| // @sap/cds/lib/srv/cds-serve.js (`if (d.is_external) srv.mocked = true`). | ||
| // Without this filter, the handler below crashes on `req.target.isDraft` | ||
| // for free-form remote calls like `srv.send(new cds.Request({ path }))`. | ||
| if (srv.mocked) continue; |
Contributor
There was a problem hiding this comment.
Can you check whether the service is external right on the service? Because remote services can exist without mocking, and relying on mocked instead may lead to confusion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR bundles two fixes and one feature for the recommendations plugin. Each is an independent commit; happy to split into separate PRs if preferred.
1. fix: exclude association elements from RPT-1 prediction rows (5e55030)
The draft branch of the recommendations READ handler builds its column list from all elements of
req.target.actives, filtering out only audit fields,cds.LargeBinaryandcds.Vector. Association/composition elements pass the filter — including the compiler-addedDraftAdministrativeDataon draft-enabled entities. These have no RPT-1 dtype (cdsToPythonDtypereturnsundefined), so they are omitted fromdata_schemabut still present inrows. Whenever other edit drafts of the entity exist, context rows resolveDraftAdministrativeDatato a non-scalar value and AI Core rejects the whole payload:The error is swallowed (empty predictions), so recommendations silently disappear while every draft READ with
$expand=SAP_Recommendationsstill pays a full failed round-trip to AI Core — observed as recurring latency on a production-adjacent stage with concurrent drafts.Repro: open an edit draft of an entity with
@UI.Recommendations, then READ another draft of that entity with$expand=SAP_Recommendations.Fix: skip
isAssociationelements when building the column list. Flattened foreign keys (author_ID, …) are separate scalar elements and remain included — they are the actual prediction targets and features. A regression test opens an edit draft (so an active context row carries draft administrative data) and asserts the rows handed tofetchPredictionscontain no association keys or non-scalar values; it fails onmainand passes with the fix.2. feat: opt-in recommendation mode (533e7ac)
Today the plugin is opt-out: every value-helped field auto-enrolls, and each unwanted one must be silenced with
@UI.RecommendationState : 0. On large existing models this enrolls dozens of fields at once and generates unnecessary/predicttraffic. New config flag:"auto"(default): unchanged behavior — absence of the flag is byte-for-byte identical to today."opt-in": a field is enrolled only if it carries a truthy@UI.RecommendationState(1or a dynamic expression); value-helped fields without it stay out.@UI.RecommendationState : 0wins in both modes; entities with zero enrolled fields get noSAP_Recommendationscompanion (existing guard). Tests boot the bookshop in opt-in mode in its own process; README and CHANGELOG updated.3. fix: skip mocked services and guard undefined
req.target(2b3fc60)cds-plugin.jsregistered the recommendations READ handler on every served service, including external services running with a mock fallback (srv.mocked). A free-form READ on such a service (e.g.srv.send(new cds.Request({ method, path }))) has noreq.target, so the handler crashed with aTypeErroronreq.target.isDraft. Mocked services are now skipped at registration, and the handler additionally bails out early whenreq.targetisundefined(defense in depth).Verification
npm test: 18/18 (node --test, sqlite bookshop +MockAICoreService), including the new 422 regression test and 3 opt-in mode testseslintandprettier --checkclean