Skip to content

Commit e0fb5cf

Browse files
committed
change versioning logic
1 parent 65a3be9 commit e0fb5cf

5 files changed

Lines changed: 17 additions & 15 deletions

File tree

.agents/skills/sdk-integrations/SKILL.md

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,21 +109,16 @@ Prefer feature detection first and version checks second.
109109
Use:
110110

111111
- `detect_module_version(...)`
112-
- `version_in_range(...)`
113-
- `version_matches_spec(...)`
114-
115-
Do not add `packaging` just for integration routing.
112+
- `version_satisfies(...)`
113+
- `make_specifier(...)`
116114

117115
## `auto_instrument()`
118116

119117
Update `py/src/braintrust/auto.py` only if the integration should be auto-patched.
120118

121-
Match the existing option shape:
122-
123-
- use plain `bool` for simple on/off integrations that do not use the integrations API
124-
- use `InstrumentOption` for integrations API providers that support `IntegrationPatchConfig`
119+
Use `InstrumentOption` (i.e. `bool | IntegrationPatchConfig`) for all integrations, including those that do not yet use the integrations API. This keeps the signature uniform and avoids a breaking change when the integration is later migrated.
125120

126-
For integrations API providers, use `_normalize_instrument_option()` and `_instrument_integration(...)` instead of adding a custom `_instrument_*` function:
121+
Use `_normalize_instrument_option()` and `_instrument_integration(...)` instead of adding a custom `_instrument_*` function:
127122

128123
```python
129124
enabled, config = _normalize_instrument_option("provider", provider)
@@ -180,6 +175,6 @@ cd py && make lint
180175
- Forgetting async or streaming coverage.
181176
- Adding patcher selection without tests for enabled and disabled cases.
182177
- Re-recording cassettes when behavior did not intentionally change.
183-
- Using `_normalize_bool_option()` for an integrations API provider.
178+
- Using `_normalize_bool_option()` instead of `_normalize_instrument_option()` — all integrations should accept `InstrumentOption`.
184179
- Adding a custom `_instrument_*` helper where `_instrument_integration()` already fits.
185180
- Forgetting `target_module` for deep or optional submodule patch targets.

py/src/braintrust/integrations/adk/__init__.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
__all__ = [
2020
"ADKIntegration",
21-
"_create_thread_wrapper",
2221
"setup_adk",
2322
"setup_braintrust",
2423
"wrap_agent",

py/src/braintrust/integrations/adk/integration.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class ADKIntegration(BaseIntegration):
2121

2222
name = "adk"
2323
import_names = ("google.adk",)
24+
min_version = "1.14.1"
2425
patchers = (
2526
ThreadBridgePatcher,
2627
AgentRunAsyncPatcher,

py/src/braintrust/integrations/test_versioning.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@ def test_version_satisfies_none_handling():
2626
assert version_satisfies("1.0", None)
2727
assert version_satisfies(None, None)
2828

29-
# No version with a spec means incompatible.
30-
assert not version_satisfies(None, ">=1.0")
29+
# No version with a spec — optimistically allow so patching still proceeds
30+
# when version detection fails.
31+
assert version_satisfies(None, ">=1.0")
3132

3233

3334
def test_version_satisfies_invalid_version():

py/src/braintrust/integrations/versioning.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,17 @@ def make_specifier(*, min_version: str | None = None, max_version: str | None =
4343

4444

4545
def version_satisfies(version: str | None, spec: str | SpecifierSet | None) -> bool:
46-
"""Return True if *version* satisfies the PEP 440 *spec*."""
46+
"""Return True if *version* satisfies the PEP 440 *spec*.
47+
48+
When *version* is ``None`` (i.e. we could not detect the installed
49+
version), we optimistically return ``True`` so that patching still
50+
proceeds. A failed detection should not silently disable
51+
instrumentation.
52+
"""
4753
if spec is None:
4854
return True
4955
if version is None:
50-
return False
56+
return True
5157
try:
5258
ss = spec if isinstance(spec, SpecifierSet) else SpecifierSet(spec, prereleases=True)
5359
return Version(version) in ss

0 commit comments

Comments
 (0)