feat(config): expose cache_stable_region_ttl_1h via ConfigField + README; gate beta header on prompt caching - #104
Merged
Conversation
…DME; gate beta header on prompt caching The `cache_stable_region_ttl_1h` knob (opts the system-prompt/tools cache breakpoints into Anthropic's 1-hour TTL) has existed since the prompt-cache breakpoint work but was never discoverable: no ConfigField advertised it via get_info(), and the README never mentioned it. Due diligence on community PR #91 (closed as superseded) surfaced this as the plausible friction behind that contribution -- the knob already did what the PR wanted, but nobody configuring the provider could find it. Changes: - amplifier_module_provider_anthropic/__init__.py: - Add a ConfigField for cache_stable_region_ttl_1h (boolean, default=None, requires_model=False, show_when gated on enable_prompt_caching) so the config wizard surfaces it, following this week's config-surface hygiene standard: an unset boolean is a real third state ("use the provider's 5-minute default"), not force-collapsed into True/False. The wizard renders a None-default boolean as "leave unset" and omits the key; the constructor already treats an absent key as False, so behavior for existing configs is unchanged. - Gate the extended-cache-ttl-2025-04-11 beta header append on enable_prompt_caching being truthy. Previously the header was sent whenever the knob was on, even with caching disabled, where it can have no effect (no cache breakpoints are ever placed). Now logs a one-line notice explaining why the knob is inert in that state. - README.md: new "Prompt Cache TTL" section documenting what the knob covers (system+tools only, by design -- conversation breakpoints stay on the 5-minute TTL, tracked separately as microsoft/amplifier#337), the write-cost economics (2x vs 1.25x, pays off once reused across the ~2+ reads a shorter TTL would have missed), and the enable_prompt_caching requirement. - tests/test_prompt_cache_breakpoints.py: new coverage for the ConfigField shape/default, the beta header being appended when knob+caching are both on (existing test, unaffected), the header being ABSENT when the knob is on but caching is off (regression test -- fails against the pre-fix code, passes after), and the absent-key default remaining False. Full suite: 665 passed (662 baseline + 3 new tests), 0 failures. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Collaborator
Author
|
All checks green (license/cla + pytest on ubuntu/macos/windows × py3.11/3.12). Merging via the maintainer admin-merge pattern: this repo requires PR review, but this is a maintainer-authored, low-risk, additive-only change (new ConfigField + gating condition + docs + tests, no changes to existing request/response code paths) opened at the direction of the repo maintainer to close a discoverability gap surfaced during due diligence on #91. No CLA or CI issues to resolve. Full local suite: 665 passed (662 baseline + 3 new), 0 failures, verified on a fresh clone of |
Brian Krabach (bkrabach)
added a commit
that referenced
this pull request
Aug 29, 2026
…ng 'false' no longer truthy (#105) Follow-up to microsoft/amplifier-module-provider-openai#74 (squash 8485663). That PR's read-only cross-check of this repo (HEAD 9916a68) found this module already has a correct boolean-parsing helper — AnthropicProvider._config_bool() (__init__.py:577-584) — used by six config keys, but FIVE boolean-ish keys bypassed it and read config with no coercion at all. The app-cli wizard writes boolean ConfigField answers as the STRING "true"/"false" (see the field_type="boolean" fields in get_info()). A plain self.config.get(key, default) returns that string unchanged, and every one of these keys is consumed in a truthiness context — any non-empty string, including the literal string "false", is truthy in Python. A user answering "false" in the wizard therefore got the feature turned ON. enable_prompt_caching is the live-reachable instance: it IS exposed as a field_type="boolean" ConfigField with string default "true" (__init__.py:975-982), so a wizard-driven "false" answer silently enabled prompt caching — and, post-#104, also fed the cache_stable_region_ttl_1h beta-header gate (if self.enable_prompt_caching: at __init__.py:812), which then misread too. Audit table (full re-audit of every self.config.get() call in the constructor, not just the 5 keys named in the task): | key | site (pre-fix) | was broken? | fixed how | |-------------------------|----------------|-------------|--------------------------------| | raw | :661 | YES — no coercion | routed through _config_bool() | | use_streaming | :748 | YES — no coercion | routed through _config_bool() | | filtered | :749-751 | YES — no coercion | routed through _config_bool() | | enable_prompt_caching | :752 | YES — no coercion; wizard-exposed boolean ConfigField, LIVE-REACHABLE | routed through _config_bool() | | enable_web_search | :753-755 | YES — no coercion | routed through _config_bool() | | retry_jitter | :676 | already safe | uses _config_bool() (pre-existing) | | fallback_on_overload | :691-693 | already safe | uses _config_bool() (pre-existing) | | enable_1m_context | :703-705 | already safe | uses _config_bool() (pre-existing) | | persist_fallback_state | :712-714 | already safe | uses _config_bool() (pre-existing) | | refusal_fallback_enabled| :720-722 | already safe | uses _config_bool() (pre-existing) | | cache_stable_region_ttl_1h | :773-775 | already safe | uses _config_bool() (pre-existing) | No other uncoerced boolean-ish config key exists in the constructor — every remaining self.config.get() call reads a numeric, string, or choice value (max_tokens, temperature, timeout, model names, thinking_type, speed, etc.), not a boolean. Fix: each of the five keys is now read as `self._config_bool(self.config.get(key, default))`, mirroring the exact call shape already used by the six safe keys. This module's _config_bool() coerces (does not fail loud on garbage — anything outside 1/true/yes/on resolves to False); no new helper was introduced, matching the module's own existing convention exactly, per the task's own guidance to not invent a new helper. Tests: tests/test_config_bool_parsing.py — 21 tests covering, per affected key: string "false" -> False, string "true" -> True, real bool passthrough, absent -> documented default; plus one integration-flavored assertion for the live-reachable key (enable_prompt_caching="false" as a string -> zero cache_control blocks in a built request). Fail-before/pass-after proof (git stash of only the source fix, keeping the new test file in place): 11 failed / 10 passed against pre-fix main (the 10 passes are the real-bool-passthrough and absent-default cases, which were never broken). Restoring the fix: all 21 pass. Full suite: 695 passed (baseline 674 passed confirmed via a clean run before touching anything, post-#103/#104, + 21 new tests = 695 exactly). No regressions. python_check on touched files: amplifier_module_provider_anthropic/__init__.py carries 15 pre-existing pyright errors + 18 pre-existing ruff-lint/stub warnings (SDK Optional-attribute narrowing, a pre-existing unsorted __all__/import block, blind-exception lint nits, a TODO stub comment) — confirmed identical (15 errors / 18 warnings, same codes and same line-number deltas as this diff's own +6 net lines) before and after this change via git stash. `ruff format`/`ruff check` show zero diff needed on this diff's own lines. The new test file is `ruff format`/`ruff check` clean; its two pyright import-resolution errors are the same isolated-file false positive every existing test file in this repo also reports when checked in isolation (verified against tests/test_prompt_cache_breakpoints.py, an unmodified pre-existing file, which reports the identical "AnthropicProvider is unknown import symbol" / "tests._helpers could not be resolved" pair) — a tool-environment artifact of checking a test file outside the project's own pytest rootdir/venv resolution, not a real defect. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
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
2 participants
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.
The gap
Found during due-diligence review of community PR #91 (closed as superseded — thank you to that contributor for surfacing the underlying friction). The provider has had a
cache_stable_region_ttl_1hconfig knob since the prompt-cache breakpoint work: it opts the system-prompt + tool-definition cache breakpoints (never the conversation region — see the design comment aboveself.cache_stable_region_ttl_1hin__init__.py) into Anthropic's 1-hour cache TTL. It already does what PR #91 was reaching for, but:ConfigField/get_info(), so the config wizard never surfaced it and nobody configuring the provider could discover it.extended-cache-ttl-2025-04-11beta header was appended whenever the knob was on, even whenenable_prompt_cachingwasFalse— a state where the header can have no effect, since no cache breakpoints are ever placed without caching enabled.What changed
amplifier_module_provider_anthropic/__init__.pyConfigFieldforcache_stable_region_ttl_1h:boolean,default=None,requires_model=False,show_when={"enable_prompt_caching": "true"}. Follows this week's config-surface hygiene standard: an unset boolean is a genuine third state ("use the provider's 5-minute default"), not force-collapsed into an explicitTrue/False. The app-cli wizard already renders aNone-default boolean field as "leave unset — use provider default" and omits the key when left blank; the constructor already treats an absent key asFalse(self.config.get("cache_stable_region_ttl_1h", False)), so behavior for existing configs is unchanged either way.extended-cache-ttl-2025-04-11beta-header append onenable_prompt_cachingbeing truthy, with a one-line log notice when the knob is set but inert.README.mdtests/test_prompt_cache_breakpoints.pytest_cache_stable_region_ttl_1h_config_field_advertised— ConfigField shape/default/show_when.test_cache_stable_region_ttl_1h_beta_header_absent_when_caching_disabled— header is appended when knob+caching are both on (pre-existing test, unaffected) and absent when the knob is on but caching is off, with the log notice asserted. Verified fail-before/pass-after against the pre-fix code.test_cache_stable_region_ttl_1h_absent_key_still_defaults_to_false— absent-key default behavior unchanged.Testing
Full suite: 665 passed (662 baseline + 3 new tests), 0 failures, on a fresh clone of
main@e64b114.Diff is scoped to
__init__.py(ConfigField + header-gating logic only) + README + tests — no changes to_cost.py, to stay clear of any concurrent cache-write TTL work there.