Skip to content

feat(aicore/fallback): add opt-in model fallback for Orchestration v2#185

Open
lenin-ribeiro wants to merge 39 commits into
mainfrom
feat/aicore-model-fallback
Open

feat(aicore/fallback): add opt-in model fallback for Orchestration v2#185
lenin-ribeiro wants to merge 39 commits into
mainfrom
feat/aicore-model-fallback

Conversation

@lenin-ribeiro

@lenin-ribeiro lenin-ribeiro commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Stacked on feat/orchestration-filtering. This branch builds on the filtering PR. Open against that branch as the base to keep the diff focused; retarget to main once filtering merges.

Description

Adds opt-in model fallback for SAP AI Core Orchestration v2 to the sap_cloud_sdk.aicore module.

Orchestration v2 supports preference-ordered fallback module configurations: when the primary call fails (model unsupported in region, 429, 408, or any 5xx — and unsupported-model only for streaming), the server transparently retries with the next preference. The underlying litellm SAP provider already builds body["config"]["modules"] as a list when fallback_sap_modules is present in optional_params; what was missing was the SDK-side ergonomic surface and the response-side visibility into which preferences were skipped.

This PR introduces:

  • FallbackModel, FallbackConfig — typed dataclasses for declaring per-preference model + params + version.
  • set_fallbacks(config) — single entry point mirroring set_filtering(). Fallback is opt-in: set_aicore_config() does NOT activate it. Developers either call set_fallbacks(...) programmatically or set AICORE_FALLBACK_ENABLED=true (with AICORE_FALLBACK_MODELS or AICORE_FALLBACK_CONFIG) and call set_fallbacks() with no args.
  • response.intermediate_failures — when the fallback path fires, the per-preference failure list from the orchestration response is surfaced as an attribute on the returned ModelResponse. None when the primary succeeded, useful as a quick check.
  • Patch composition — the existing FilteringOrchestrationConfig is renamed to OrchestrationPatchConfig (alias kept for back-compat) and now owns both filtering and fallback. One install/uninstall path, no ordering issues.
  • Filtering broadcast across fallbacks — when both filtering and fallback are active, the filtering config is applied to every module entry on the wire (previously modules[0] only). Consistent SDK-side default; if a fallback should run unfiltered, the developer can call disable_filtering() before the call.

Related Issue

N/A — additive feature, no issue tracked

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Code refactoring

How to Test

Unit tests (no live credentials required)

uv run python -m pytest tests/aicore -v

Expect 142 passed, 8 skipped (the 8 skips are integration scenarios waiting for live env vars).

Integration tests (requires AI Core access)

  1. Copy .env_integration_tests.example to .env_integration_tests and fill in the AI Core creds.
  2. Set the new fallback secrets:
    AICORE_FALLBACK_TEST_PRIMARY_MODEL=sap/this-model-does-not-exist
    AICORE_FALLBACK_TEST_FALLBACK_MODEL=sap/mistralai--mistral-small-instruct
    The primary should be a model name the orchestration server reports as unsupported in your region — a genuinely nonexistent name works and avoids depending on transient 5xx errors.
  3. Run:
    uv run python -m pytest tests/aicore/integration/test_fallback_bdd.py -v
  4. Expected: 4 scenarios pass (primary succeeds, primary unsupported → fallback used, filtering + fallback composition, streaming + fallback).

Manual smoke

from sap_cloud_sdk.aicore import (
    FallbackConfig, FallbackModel, set_aicore_config, set_fallbacks,
)
from litellm import completion

set_aicore_config()
set_fallbacks(FallbackConfig([
    FallbackModel(model="sap/mistralai--mistral-small-instruct"),
]))

response = completion(
    model="sap/gpt-4o",  # or any potentially-unsupported model
    messages=[{"role": "user", "content": "Translate 'hello' to German."}],
)
print(response.choices[0].message.content)
print(getattr(response, "intermediate_failures", None))
# None if primary worked; list of failure dicts if fallback fired.

Checklist

  • I have read the Contributing Guidelines
  • I have verified that my changes solve the issue
  • I have added/updated automated tests to cover my changes
  • All tests pass locally (pytest tests/aicore, ruff check, ruff format --check, ty check)
  • I have verified that my code follows the Code Guidelines
  • I have updated documentation (if applicable) — aicore/user-guide.md gains a "Model Fallback (opt-in)" section
  • I have added type hints for all public APIs
  • My code does not contain sensitive information (credentials, tokens, etc.)
  • I have followed Conventional Commits for commit messages

Breaking Changes

None for users of sap_cloud_sdk.aicore public APIs. All existing names (FilteringOrchestrationConfig, _install, etc.) remain importable via aliases.

There is one user-visible behavioural change that only affects users who use filtering AND fallback together (an impossible combination on main today since fallback didn't exist): when both are active, the filtering configuration now applies to every module entry (primary + every fallback), not just modules[0]. This is the safe-by-default semantic — to run a fallback unfiltered, explicitly disable_filtering() before that call. Documented in the user guide.

Additional Notes

Design choices (selected via user Q&A during planning)

  • API style — patch-based, mirrors filtering. Single set_fallbacks() entry point, global state, no disable_fallbacks() (developers either opt in via set_fallbacks() or never call it; runtime clearing is set_fallbacks(None)).
  • One subclass for both concerns. OrchestrationPatchConfig handles filtering injection AND fallback injection in one transform_request. Single install/uninstall lifecycle. Idempotent.
  • Env vars opt-in. AICORE_FALLBACK_ENABLED defaults to false (unlike filtering, which is on by default after set_aicore_config()). Two-tier schema: AICORE_FALLBACK_MODELS (comma list, simple case) + AICORE_FALLBACK_CONFIG (JSON, full per-model config; takes precedence).
  • intermediate_failures on the response object. Pydantic ModelResponse uses extra="allow", so we can attach the field directly. Accessed via getattr(response, "intermediate_failures", None).

v1 limitations (documented)

  • intermediate_failures is surfaced for non-streaming responses only. Capturing the field from SAPStreamIterator chunks requires deeper changes to litellm internals and is deferred to a future iteration. The streaming integration test asserts that fallback still fires correctly server-side; it doesn't assert intermediate_failures.

Telemetry

Added Operation.AICORE_SET_FALLBACKS = "set_fallbacks". The set_fallbacks entry point is decorated with @record_metrics(Module.AICORE, Operation.AICORE_SET_FALLBACKS) per docs/GUIDELINES.md.

Files added / changed

NEW:
  src/sap_cloud_sdk/aicore/fallback/__init__.py
  src/sap_cloud_sdk/aicore/fallback/fallback.py
  tests/aicore/fallback/__init__.py
  tests/aicore/fallback/unit/__init__.py
  tests/aicore/fallback/unit/test_fallback_config.py
  tests/aicore/fallback/unit/test_patch.py
  tests/aicore/fallback/unit/test_set_fallbacks.py
  tests/aicore/integration/fallback.feature
  tests/aicore/integration/test_fallback_bdd.py

MODIFIED:
  src/sap_cloud_sdk/aicore/__init__.py            # export FallbackModel/FallbackConfig/set_fallbacks; docstring
  src/sap_cloud_sdk/aicore/filtering/filters.py   # OrchestrationPatchConfig rename + split _install + broadcast filtering + attach intermediate_failures
  src/sap_cloud_sdk/aicore/user-guide.md          # new "Model Fallback (opt-in)" section
  src/sap_cloud_sdk/core/telemetry/operation.py   # AICORE_SET_FALLBACKS
  tests/aicore/integration/conftest.py            # extend with fallback fixtures + clean-skip behaviour
  tests/core/unit/telemetry/test_operation.py     # expected enum count 152 → 153
  .env_integration_tests.example                  # AICORE_FALLBACK_TEST_PRIMARY_MODEL / _FALLBACK_MODEL

Activates Azure Content Safety filtering and prompt attack detection
automatically for all SAP AI Core model calls. Filtering is enabled
by default when set_aicore_config() is called — no code change required
by the developer.

- New module sap_cloud_sdk.orchestration with:
  - FilteringModuleConfig: configures input/output filtering thresholds
    and prompt shield via ORCH_FILTER_* env vars (defaults: threshold 4,
    prompt_shield=True on input)
  - set_filtering(): programmatic override for thresholds at runtime
  - ContentFilteredError: raised when input or output is rejected by
    the content filter
  - extract_filter_blocked(): unwraps filter rejections embedded in
    LiteLLM APIConnectionError exceptions
- set_aicore_config() now calls _activate_filtering() at the end,
  applying FilteringModuleConfig.from_env() to LiteLLM's SAP provider
- Observability preserved: LiteLLM still makes the HTTP call;
  Traceloop/OTel instrumentation is unaffected
- 41 unit tests covering serialisation, env parsing, LiteLLM patch,
  response detection, and set_filtering() behaviour
- User guides updated in aicore/ and orchestration/; README breaking
  change notice added
- Version bump 0.27.1 → 0.28.0
read_env_str / read_env_bool / read_env_choice — used by the
filtering module for AICORE_FILTER_* runtime toggles. Distinct
from secret_resolver, which handles credential mount-and-fallback.
File moves only; imports are fixed in following commits.
Tests will fail at this commit and pass once Task 6 lands.
…env vars

- Replace Literal[0,2,4,6] with Severity IntEnum (STRICT/LOW/MEDIUM/OFF)
- Rename ORCH_FILTER_* -> AICORE_FILTER_* env vars
- Drop local _env/_env_bool/_env_severity helpers; use core.env
Update test imports and @patch() string targets to match the new
sap_cloud_sdk.aicore.filtering location. Rename ORCH_FILTER -> AICORE_FILTER
in test fixtures.
- Drop Module.ORCHESTRATION (14 modules -> 13)
- Drop ORCHESTRATION_SET_FILTERING operation
- Add AICORE_SET_FILTERING, AICORE_DISABLE_FILTERING,
  AICORE_EXTRACT_FILTER_BLOCKED under # AI Core Operations
- Update test_operation.py operation count (150 -> 152)
- Replace set_filtering(enabled=False) with dedicated disable_filtering()
- Add @record_metrics(AICORE, EXTRACT_FILTER_BLOCKED) on the parser
- Retarget set_filtering telemetry to Module.AICORE/AICORE_SET_FILTERING
- set_filtering args typed as Severity instead of Literal[0,2,4,6]
- Re-export set_filtering, disable_filtering, Severity, and exception
  types from sap_cloud_sdk.aicore
- Drop lazy import / try-except in set_aicore_config: no longer a
  circular-dep risk and any failure should surface, not be swallowed
- Update set_aicore_config docstring to describe filtering side effect
- Rename ORCH_FILTER_ENABLED -> AICORE_FILTER_ENABLED
- set_filtering(enabled=False) -> disable_filtering()
- Link points at aicore user guide content-filtering section
- Update env-var table to AICORE_FILTER_*
- Replace set_filtering(enabled=False) examples with disable_filtering()
- Show Severity enum usage in code examples
- Add Migration section showing the rename from sap_cloud_sdk.orchestration
- Update import paths to sap_cloud_sdk.aicore in all examples
- _litellm_patch: 'orchestration filtering' log msgs -> 'content filtering';
  remove unused # type: ignore[attr-defined] suppressions (ty no longer needs them)
- exceptions: module docstring + OrchestrationError docstring refer to the
  aicore.filtering location and clarify the class still serves as the base
  for orchestration-module errors (filtering today, grounding/masking later)
…t shield

Live tests against an AI Core orchestration endpoint with Azure Content
Safety. Skips cleanly when AICORE_* env vars are absent. Four scenarios:
OFF baseline, ON benign, input-filter STRICT block, Prompt Shield
jailbreak block.

The jailbreak prompt is taken verbatim from Microsoft Learn's Prompt
Shields documentation; the self-harm prompt is left as an empty
placeholder so operators can populate it from internal red-team fixtures
without committing harmful content to the public repository.
CI Code Quality job flagged these on commit 8a9babc:

- Ruff format: aicore/__init__.py, core/env.py, filtering/_models.py and
  several test files needed reformatting per the project's ruff config.
  Apply 'uv run ruff format' to all affected files. Drops unused imports
  (MagicMock, Mock, pytest) in tests/aicore/unit/test_aicore.py.

- ty check: 'cast: Callable[[str], T] = int  # type: ignore[assignment]'
  in core/env.py was rejected by CI's stricter ty. Drop the cast kwarg
  entirely (YAGNI — no caller passes anything other than int) and tighten
  TypeVar bound to int. Also resolves the integration test 'ctx.response
  has no attribute choices' error by typing ScenarioContext.response as Any.

- Wire AICORE_FILTER_TEST_SELF_HARM_PROMPT environment variable into the
  integration test's AZURE_TEST_PROMPTS dict so the operator can populate
  the self-harm scenario via GitHub secret without committing harmful
  content to the public repo. Scenario still skips when the var is unset.
… format

- core/env.py: drop the TypeVar from read_env_choice — CI ty rejects the
  T→int return-value coercion even with a type-ignore directive. The only
  caller (filtering/_models.py) wraps the result in Severity() anyway, so
  the generic typing was dead weight. Function now returns plain int.

- integration test 'Prompt Shield blocks a jailbreak attempt' was passing
  the request to Azure correctly (ContentFilteredError raised, direction
  input, request_id present) but my assertion looked for substring
  'prompt_shield' / 'jailbreak' in the details payload. Azure's actual
  wire format is:
      {'azure_content_safety': {'user_prompt_analysis': {'attack_detected': True}}}
  Rewrite the assertion (and the feature-file step) to check the
  structured 'attack_detected: True' flag — robust to wording shifts in
  Azure's response and a more meaningful semantic check.
…uard38bFilter

Provider classes mirroring the gen_ai_hub.orchestration.models shape:
- ContentFilter abstract base with to_dict() that emits {type, config}
- AzureContentFilter: 4 severity categories + optional prompt_shield
- LlamaGuard38bFilter: 14 boolean category flags
Wire-format unchanged from FilteringModuleConfig path; this lays the
groundwork for replacing the kwarg-driven set_filtering API.
…iltering

Direction containers + top-level configuration mirroring gen-ai-hub-sdk:
- InputFiltering(filters=[...]) and OutputFiltering(filters=[...], stream_options=...)
- ContentFiltering(input_filtering=..., output_filtering=...) with to_dict()
- ContentFiltering.from_env() reads AICORE_FILTER_* env vars and builds
  a single AzureContentFilter per direction (replaces
  FilteringModuleConfig.from_env). prompt_shield applied only to input
  direction, matching server semantics.
Coupled changes that must land together (Tasks 3-6 of the plan):

- _models.py shrinks to just the Severity enum; ContentFilterConfig,
  PromptShieldConfig, FilteringModuleConfig are removed in favour of
  the class hierarchy added in earlier commits (_filters.py, _modules.py).
- _litellm_patch.py: _install annotation comment now reads
  'ContentFiltering | None'. Runtime type stays Any to avoid a circular
  import.
- aicore/filtering/__init__.py: set_filtering(config: ContentFiltering | None)
  takes a single positional arg. Old kwargs (hate, violence, sexual,
  self_harm, prompt_shield, directions) are removed. __all__ now exports
  the new 9-name class API; the previously-exposed ContentFilterConfig
  / PromptShieldConfig / FilteringModuleConfig are gone.
- aicore/__init__.py: re-exports updated; 13 public names total.
- test_filtering.py rewritten for the class API: TestSetFiltering covers
  no-args / explicit-None / explicit config / env-disable / explicit-config
  override / multi-filter cases. TestDisableFiltering unchanged.
- test_patch.py: TestInstall fixtures and TestTransformRequest cases
  build ContentFiltering objects instead of FilteringModuleConfig.
- test_models.py shrinks to TestSeverity (3 tests).

All 749 unit tests pass + 4 integration skipped. Wire format unchanged;
the live AI Core orchestration v2 endpoint sees identical JSON.
filtering_strict() and filtering_prompt_shield() now build
ContentFiltering(InputFiltering(filters=[AzureContentFilter(...)]))
and pass it to set_filtering. filtering_default() (no args) is
unchanged.
- Programmatic override now uses ContentFiltering / InputFiltering /
  OutputFiltering / AzureContentFilter
- New Multiple filter providers subsection introduces LlamaGuard38bFilter
- Migration subsection covers both the orchestration->aicore rename and
  the kwarg->class API shift
ty doesn't infer that 'assert cfg is not None' narrows cfg.input_filtering
from 'InputFiltering | None' to 'InputFiltering'. Add the explicit second
assertion in the two cases where we then access .filters[0] so the static
type-check passes alongside the dynamic check.
Address two review comments from @NicoleMGomes on the post-restructure
PR state:

- README.md: drop the breaking-change callout (review #9). The PR body
  + aicore user-guide carry the relevant migration detail; the README
  doesn't need release-notes-style content per change.
- _litellm_patch.py: tighten 'except Exception: pass' to 'except ValueError'
  around the json() call only (review #10). The previous broad except
  would swallow logic bugs in ContentFilteredError(...) construction;
  the narrower catch handles only the realistic failure mode
  (non-JSON response body) and lets logic errors surface. Applied
  symmetrically to both input-filter and output-filter detection
  branches.

Also satisfy CI's stricter ty by adding 'ty: ignore[too-many-positional-arguments]'
to two deliberately-failing positional-call sites in test_filters.py
(test_kwarg_only). The existing 'type: ignore[misc]' was sufficient for
mypy but not for the CI ty version.
- README: drop the 'Content Filtering & Prompt Shield' bullet from
  Key Features. It's a sub-feature of 'AI Core Integration' already
  on the list; other modules don't fan out their sub-features either.
- Move set_filtering / disable_filtering bodies from filtering/__init__.py
  to a new filtering/_api.py. filtering/__init__.py is now a thin
  re-export surface; logic lives in named modules.
- Inline Severity into _filters.py (where its only consumer
  AzureContentFilter lives) and delete _models.py — a one-enum file
  with a misleading 'models' name. TestSeverity moves to test_filters.py.

749 unit tests pass; flat 13-name public API at sap_cloud_sdk.aicore
is unchanged for external callers.
Address review comment #14 ("can we have all public methods on a
filters.py? Better to read").

Merge _filters.py + _modules.py + _api.py into a single filters.py
(no underscore — signals the public surface). It now owns Severity,
ContentFilter, AzureContentFilter, LlamaGuard38bFilter, InputFiltering,
OutputFiltering, ContentFiltering, set_filtering, disable_filtering.
Tests import directly from .filters; the package __init__ stays a thin
re-export for the flat 'from sap_cloud_sdk.aicore import ...' path.

Internal-only files (_litellm_patch, exceptions) keep their separate
locations; they aren't part of the documented public surface.
CI's ty check started reporting 6 'unused ty: ignore directive' warnings
on b594bd6 that didn't appear on 8bcd53b — same ty==0.0.21 binary, but
the diagnostic count is non-zero and the pre-commit hook fails the build.

Drop the dead 'ty: ignore[...]' suppressions on pre-existing test sites
that ty no longer flags as errors:

- tests/adms/unit/test_client.py: 4 sites, MagicMock method-assign and
  union-attr patterns. The accompanying '# type: ignore[...]' for mypy
  is kept because mypy still needs it; only the ty annotation is removed.
- tests/adms/unit/test_http.py: 1 site, invalid-argument-type passing None
  to a strict-typed parameter inside pytest.raises.
- tests/adms/unit/test_query_options.py: 1 site, unknown-argument inside
  pytest.raises.
- tests/aicore/filtering/unit/test_filters.py: 2 sites, the
  too-many-positional-arguments directives I added earlier when CI's ty
  rejected deliberately-failing positional constructor calls — ty no
  longer flags these, so the suppressions are dead.

tests/destination/unit/test_client.py keeps its 2 'ty: ignore[invalid-argument-type]'
directives — ty still flags those as real errors there; the suppression
is doing genuine work to mask a deliberate test of validation rejection.
Code Quality CI runs 'uvx ty check .' which evaluates suppressions
differently from 'uv run ty check .' — same ty==0.0.21 binary but
different cache state. After commit 3e3edb7, CI reported 6 errors at
sites where I had removed 'ty: ignore[...]' directives but 'uv run ty'
locally said the directives were unused. After re-adding them, CI then
reported them as unused warnings — flapping.

Resolution: matched CI's invocation exactly ('uvx ty check .'), removed
suppressions where uvx ty flagged them as unused-ignore, kept (re-added
already in 3e3edb7) suppressions where uvx ty would flag the underlying
code as error. Verified locally with the same uvx command CI uses.

Sites cleared (suppressions no longer needed):
- tests/adms/unit/test_client.py (4 sites)
- tests/destination/unit/test_client.py (2 sites)
- tests/aicore/filtering/unit/test_filters.py (2 sites, my own additions)

Sites kept (suppressions still needed):
- tests/adms/unit/test_http.py:274 (invalid-argument-type)
- tests/adms/unit/test_query_options.py:86 (unknown-argument)

Also includes uv.lock SDK version bump 0.27.0 -> 0.28.0 (regenerated
from pyproject.toml; previously dirty in working tree).
…ers.py

Address review comments #17 and #18 (part 1):

- Merge _litellm_patch.py (LiteLLM transport patch, _install, _active_cfg,
  _ORIGINAL_CONFIG, FilteringOrchestrationConfig, extract_filter_blocked)
  into filters.py. The whole filtering surface (Severity, ContentFilter,
  AzureContentFilter, LlamaGuard38bFilter, InputFiltering, OutputFiltering,
  ContentFiltering, set_filtering, disable_filtering, extract_filter_blocked,
  FilteringOrchestrationConfig) now lives in one file at ~565 lines.
- Inline _read_env_str / _read_env_bool / _read_env_choice helpers from
  core/env.py into filters.py (private module-level helpers). Delete
  src/sap_cloud_sdk/core/env.py + tests/core/unit/test_env.py — single
  consumer, no need for a shared module yet.

aicore/filtering/__init__.py stays a thin re-export of the public names
plus the exception types. exceptions.py is the only other file in the
package.

Test imports updated: test_filtering.py and test_patch.py now import
_install / _ORIGINAL_CONFIG / FilteringOrchestrationConfig from
.filters (not ._litellm_patch). Mock paths in test_patch.py updated to
match the new module location.
- Add py.typed markers to aicore/ and aicore/filtering/ packages so type
  checkers consuming the SDK see the inline annotations (PEP 561).
- Document AICORE_FILTER_TEST_MODEL and AICORE_FILTER_TEST_SELF_HARM_PROMPT
  in .env_integration_tests.example alongside the existing AICORE_*
  entries, with a comment explaining the self-harm prompt is operator-
  populated and kept out of source.
- Narrow extract_filter_blocked's catch-all 'except Exception' to
  '(ValueError, KeyError, TypeError, AttributeError)'. Documents the
  realistic failure modes (JSON parse, missing key, wrong shape, attr on
  non-object) and stops swallowing logic bugs in ContentFilteredError
  construction.
Add a sibling 'fallback' subpackage exposing FallbackModel, FallbackConfig
(dataclasses) and a set_fallbacks() entry point that mirrors the filtering
API style. Fallback is opt-in: set_aicore_config() does not enable it; the
developer activates it explicitly via set_fallbacks() or by setting
AICORE_FALLBACK_ENABLED=true and AICORE_FALLBACK_MODELS / _CONFIG.

The litellm SAP provider already builds modules as a list when
fallback_sap_modules is present in optional_params. The SDK now injects
that kwarg from the active FallbackConfig via the existing transport patch.

Refactor filtering/filters.py to host both concerns in a single subclass,
OrchestrationPatchConfig (FilteringOrchestrationConfig kept as alias):

- _install split into _install_filter + _install_fallback sharing
  _apply_patch(); _install retained as alias for back-compat.
- transform_request now injects fallback_sap_modules before super(), and
  BROADCASTS filtering to every module entry (was modules[0] only).
- transform_response attaches response.intermediate_failures from the body
  so callers can inspect which preferences were skipped. Non-streaming only
  in v1; streaming surfacing is deferred.

Tests:
- 35 new unit tests across test_fallback_config.py, test_patch.py and
  test_set_fallbacks.py covering dataclass shape, env parsing, patch
  injection, filtering broadcast, intermediate_failures attachment, and
  install lifecycle composition with filtering.
- New BDD fallback.feature + test_fallback_bdd.py with 4 scenarios
  (primary success, primary unsupported -> fallback used, filtering
  composition, streaming + fallback). conftest skips cleanly when
  AICORE_FALLBACK_TEST_* env vars are missing.
- Bump expected enum count for AICORE_SET_FALLBACKS.

Docs & ops:
- user-guide.md gains a Model Fallback (opt-in) section with programmatic
  and env-driven examples, composition with filtering, and the v1
  streaming limitation.
- .env_integration_tests.example documents the new
  AICORE_FALLBACK_TEST_PRIMARY_MODEL / _FALLBACK_MODEL secrets.
@lenin-ribeiro lenin-ribeiro self-assigned this Jun 24, 2026
@lenin-ribeiro lenin-ribeiro requested a review from a team as a code owner June 24, 2026 08:38
lenin-ribeiro and others added 8 commits June 24, 2026 11:19
…odule entries

litellm's transform_request only builds the primary module's template from
`messages`; fallback entries get whatever was popped from their dict's
"messages" key (transformation.py:371), which is `[]` for
FallbackModel.to_dict(). The orchestration server then rejected with
"config.modules[N].prompt_templating.prompt.template should be non-empty".

Mirrors the existing filtering broadcast in the same transform_request.

Adds a realistic unit test (and helper) that would have caught this before
integration — the previous list-modules fixture hardcoded an empty
template on both the primary AND fallback entries, normalising the bug away.
Parity with sibling subpackages aicore/ and aicore/filtering/, both of
which already ship a py.typed marker. The parent package marker already
covers the subpackage transitively, but the one-marker-per-subpackage
convention is what docs/GUIDELINES.md prescribes.
main advanced to 0.29.0 since we last rebased (commit 101492b on main).
Our 0.28.1 would fail the 'Check Version Bump' CI job which requires
strictly greater than main.

Going to 0.30.0 (minor) rather than 0.29.1 (patch) — this PR adds
substantive new public API (ContentFiltering / InputFiltering /
OutputFiltering / AzureContentFilter / LlamaGuard38bFilter classes;
set_filtering / disable_filtering entry points; Severity enum;
AICORE_FILTER_* env-var protocol). Patch-version connotes bugfix-only,
which understates the surface change.
Address final review comment: 'filters.py file is too bloated. We should
separate concerns, like create a config.py and put the env var fetching
code, and models.py for the classes.'

Match the sibling-module convention (agent_memory, destination, etc.):

- _models.py: Severity enum + provider classes (ContentFilter,
  AzureContentFilter, LlamaGuard38bFilter) + direction containers
  (InputFiltering, OutputFiltering, ContentFiltering). No behaviour
  beyond construction and to_dict() serialisation.
- config.py: load_from_env() function + private _read_env_* helpers.
  Replaces ContentFiltering.from_env() classmethod with a module-level
  factory, matching agent_memory/config.py's load_from_env_or_mount()
  shape.
- _patch.py: FilteringOrchestrationConfig, _install, _active_cfg,
  _ORIGINAL_CONFIG — the LiteLLM transport monkeypatch.
- _api.py: set_filtering, disable_filtering, extract_filter_blocked
  decorated with @record_metrics. Honours the earlier review #12
  ('not good pattern to keep logic on init').
- __init__.py: thin re-export surface (the 12 public names) +
  module-layout doc.

filters.py deleted. Test imports updated to the new module paths;
mock paths in test_patch.py point at _patch.GenAIHubOrchestrationConfig.
ContentFiltering.from_env() classmethod removed in favour of
load_from_env() — callers (tests + _api.py) updated accordingly.
CI's uvx-installed ruff (latest) enforces a tighter line length than
the project's pinned uv-run ruff. One line in load_from_env reformatted
to wrap the Severity(_read_env_choice(...)) call across three lines,
matching the surrounding violence/sexual/self_harm sites.
…d filtering package

The parent branch refactored aicore/filtering/filters.py into four files
(_api.py, _models.py, _patch.py, config.py). This branch's fallback code
hooked directly into filters.py; the merge requires porting:

- src/sap_cloud_sdk/aicore/fallback/_patch.py (new): owns
  OrchestrationPatchConfig (now a subclass of FilteringOrchestrationConfig),
  _active_fallback_cfg, and _install_fallback. Keeps the same hooks as
  before: fallback_sap_modules injection, prompt-template broadcast to
  every fallback module entry, filtering broadcast across all entries
  (overriding the parent's primary-only injection), intermediate_failures
  attachment.

- src/sap_cloud_sdk/aicore/filtering/_patch.py: _install now defers to the
  installed fallback subclass when _active_fallback_cfg is set, so calling
  set_filtering() while fallback is active no longer clobbers the patch.
  Lazy import of fallback._patch avoids a circular dependency.

- src/sap_cloud_sdk/aicore/fallback/fallback.py: import _install_fallback
  from the new ._patch module instead of the deleted filtering.filters.

- tests/aicore/fallback/unit/test_patch.py + test_set_fallbacks.py: rewired
  to the new import paths. Adjusted test_patch_installed_when_only_filtering
  to assert FilteringOrchestrationConfig (not OrchestrationPatchConfig) is
  installed — filtering-only no longer uses the combined subclass under the
  refactored design.

Local verification: pytest tests/aicore → 145 passed, 8 skipped; pytest
tests (sans live-credential integration suites) → 2610 passed, 73 skipped;
ruff check + ruff format --check + ty check → all green.
Base automatically changed from feat/orchestration-filtering to main June 30, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant