Improve interface #11
Conversation
|
Warning Rate limit exceeded@btucker has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis PR removes the Client API, adds BaseSession and AsyncSession, unifies generation into a single sync/async generate(prompt, schema, stream) interface, introduces GenerationResponse and StreamChunk types, changes tool registration/transcript handling, and deletes legacy language/stats/message helpers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Session
participant BaseSession
participant FFI as _foundationmodels (FFI)
User->>Session: generate(prompt, schema=None, stream=False)
Session->>BaseSession: _check_closed()
BaseSession-->>Session: ok
Session->>BaseSession: _begin_generation()
BaseSession-->>Session: start_length
alt schema provided
Session->>FFI: generate_structured(prompt, schema_json)
else text
Session->>FFI: generate(prompt, params)
end
FFI-->>Session: content (str or dict)
Session->>BaseSession: _end_generation(start_length)
BaseSession->>BaseSession: extract transcript delta & tool calls
Session->>BaseSession: _build_generation_response(content, is_structured, start_length)
BaseSession-->>Session: GenerationResponse
Session-->>User: return GenerationResponse
sequenceDiagram
autonumber
participant User
participant AsyncSession
participant BaseSession
participant Worker as ThreadPool/Worker
participant Queue as asyncio.Queue
User->>AsyncSession: async for chunk in generate(..., stream=True)
AsyncSession->>BaseSession: _check_closed()
BaseSession-->>AsyncSession: ok
AsyncSession->>BaseSession: _create_stream_queue()
BaseSession-->>AsyncSession: asyncio.Queue
AsyncSession->>BaseSession: create stream callback --> callback pushes to Queue
par background thread
Worker->>FFI: generate_stream(prompt, callback)
loop streaming
FFI->>Worker: chunk_str
Worker->>Queue: callback(chunk_str)
end
Worker->>Queue: callback(None) [sentinel]
and main event loop
AsyncSession->>Queue: await queue.get()
Queue-->>AsyncSession: chunk_str
AsyncSession-->>User: yield StreamChunk
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/test_integration.py (1)
256-277: Fix response string handling before printing.After the switch to
GenerationResponse, slicing (response[:50]) andlen(response)now raiseTypeError, so these branches fall into the exception path even when generation succeeded. Capture the text returned byassert_valid_responseand operate on that string instead. citeturn0pipeline0- assert_valid_response(response) - print(f"Response: {response[:50]}...") + response_text = assert_valid_response(response) + print(f"Response: {response_text[:50]}...") print("✓ Empty prompt handled gracefully") empty_handled = True @@ - assert_valid_response(response) - print(f"✓ Long prompt handled ({len(response)} chars in response)") + response_text = assert_valid_response(response) + print(f"✓ Long prompt handled ({len(response_text)} chars in response)") long_handled = Trueapplefoundationmodels/__init__.py (2)
58-58: Update__version__to match the release.PyPI metadata now advertises 0.2.0, but
applefoundationmodels.__version__still reports 0.1.0. Please bump the constant here to stay consistent. citeturn0file0-__version__ = "0.1.0" +__version__ = "0.2.0"
62-107: Avoid circular import when exporting async APIs.Importing
applefoundationmodelsnow raisesImportError: cannot import name '_foundationmodels' … (partially initialized module)in CI because the new module-levelfrom .async_client import AsyncClientcompletes a cycle while the package is still loading. Please switch to a lazy export (e.g., gate the import behindTYPE_CHECKINGand expose it via__getattr__) or otherwise break the cycle so the base package can be imported again. citeturn0pipeline0
🧹 Nitpick comments (2)
applefoundationmodels/async_session.py (2)
144-146: Consider using a custom exception class.The static analysis tool suggests defining a custom exception class rather than specifying long messages inline. However, this is minor for a clear validation error like this.
224-232: Consider more efficient async coordination pattern.The current polling loop uses
asyncio.sleep(0)to yield control andqueue.get(timeout=0.1)which results in busy-waiting with 100ms intervals. This works but is not optimal for async code.Consider using
asyncio.Queuewithawait queue.get()instead:# Use asyncio.Queue instead of Queue queue: asyncio.Queue = asyncio.Queue() def callback(chunk: Optional[str]) -> None: asyncio.run_coroutine_threadsafe(queue.put(chunk), loop) # Get the event loop before starting thread loop = asyncio.get_event_loop() # ... thread setup ... # Then in the iteration: while True: item = await queue.get() # No timeout needed, truly async # ... rest of logic ...This eliminates polling and provides true async coordination between the background thread and async iterator.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
README.md(9 hunks)applefoundationmodels/__init__.py(6 hunks)applefoundationmodels/_foundationmodels.pyi(0 hunks)applefoundationmodels/_foundationmodels.pyx(3 hunks)applefoundationmodels/async_client.py(1 hunks)applefoundationmodels/async_session.py(1 hunks)applefoundationmodels/base_client.py(1 hunks)applefoundationmodels/base_session.py(1 hunks)applefoundationmodels/client.py(4 hunks)applefoundationmodels/session.py(4 hunks)applefoundationmodels/types.py(2 hunks)examples/basic_chat.py(0 hunks)pyproject.toml(1 hunks)tests/conftest.py(2 hunks)tests/test_client.py(0 hunks)tests/test_integration.py(5 hunks)tests/test_session.py(16 hunks)tests/test_tools.py(6 hunks)
💤 Files with no reviewable changes (3)
- tests/test_client.py
- applefoundationmodels/_foundationmodels.pyi
- examples/basic_chat.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-09T03:38:10.462Z
Learnt from: btucker
Repo: btucker/apple-foundation-models-py PR: 7
File: setup.py:117-165
Timestamp: 2025-11-09T03:38:10.462Z
Learning: In the apple-foundation-models-py repository, the UniversalWheel class in setup.py intentionally overrides get_tag() to return 'py3-none-any' as a temporary workaround until PyPI supports macosx-26 wheels. This is an accepted tradeoff with runtime checks in Client.__init__() to handle unsupported platforms.
Applied to files:
pyproject.toml
🧬 Code graph analysis (11)
applefoundationmodels/async_client.py (4)
applefoundationmodels/base_client.py (4)
BaseClient(17-151)_validate_platform(28-54)_initialize_library(57-65)_build_session_config(117-151)applefoundationmodels/async_session.py (2)
AsyncSession(38-278)close(56-58)applefoundationmodels/client.py (2)
close(45-53)create_session(55-89)applefoundationmodels/_foundationmodels.pyi (1)
create_session(16-16)
tests/test_session.py (6)
applefoundationmodels/types.py (3)
text(229-243)parsed(246-260)parse_as(262-285)tests/conftest.py (3)
session(30-35)check_availability(10-18)client(22-26)applefoundationmodels/async_session.py (4)
generate(62-69)generate(73-80)generate(84-91)generate(93-156)applefoundationmodels/session.py (4)
generate(60-67)generate(71-78)generate(82-89)generate(91-154)applefoundationmodels/client.py (2)
client(93-109)create_session(55-89)applefoundationmodels/base_session.py (2)
transcript(160-183)last_generation_transcript(186-214)
applefoundationmodels/base_client.py (5)
applefoundationmodels/base.py (1)
ContextManagedResource(13-36)applefoundationmodels/types.py (1)
Availability(52-64)applefoundationmodels/exceptions.py (1)
NotAvailableError(26-29)applefoundationmodels/_foundationmodels.pyi (6)
init(6-6)check_availability(11-11)get_availability_reason(12-12)is_ready(13-13)get_version(8-8)register_tools(42-42)applefoundationmodels/tools.py (2)
extract_function_schema(153-233)attach_tool_metadata(236-268)
tests/test_integration.py (4)
applefoundationmodels/types.py (1)
text(229-243)tests/conftest.py (4)
assert_valid_response(41-68)session(30-35)client(22-26)assert_valid_chunks(71-96)applefoundationmodels/async_session.py (5)
generate(62-69)generate(73-80)generate(84-91)generate(93-156)close(56-58)applefoundationmodels/async_client.py (3)
AsyncClient(14-97)create_session(60-97)close(50-58)
applefoundationmodels/__init__.py (3)
applefoundationmodels/async_client.py (1)
AsyncClient(14-97)applefoundationmodels/async_session.py (1)
AsyncSession(38-278)applefoundationmodels/types.py (2)
GenerationResponse(192-285)StreamChunk(289-310)
tests/test_tools.py (5)
applefoundationmodels/client.py (3)
client(93-109)create_session(55-89)Client(16-89)tests/conftest.py (2)
client(22-26)session(30-35)applefoundationmodels/_foundationmodels.pyi (2)
create_session(16-16)generate(19-19)applefoundationmodels/types.py (1)
text(229-243)applefoundationmodels/base_session.py (1)
transcript(160-183)
applefoundationmodels/base_session.py (3)
applefoundationmodels/base.py (1)
ContextManagedResource(13-36)applefoundationmodels/types.py (5)
NormalizedGenerationParams(102-138)GenerationResponse(192-285)ToolCall(164-188)Function(142-160)from_optional(118-138)applefoundationmodels/_foundationmodels.pyi (1)
get_transcript(43-43)
applefoundationmodels/async_session.py (4)
applefoundationmodels/base_session.py (7)
BaseSession(20-214)_check_closed(42-50)_normalize_generation_params(52-65)_begin_generation(67-74)_build_generation_response(112-157)_end_generation(76-83)transcript(160-183)applefoundationmodels/types.py (4)
NormalizedGenerationParams(102-138)GenerationResponse(192-285)StreamChunk(289-310)text(229-243)applefoundationmodels/pydantic_compat.py (1)
normalize_schema(116-144)applefoundationmodels/_foundationmodels.pyi (5)
generate(19-19)generate_structured(22-27)generate_stream(30-35)get_history(38-38)clear_history(39-39)
tests/conftest.py (3)
tests/test_tools.py (2)
session(85-89)client(78-81)applefoundationmodels/client.py (2)
client(93-109)create_session(55-89)applefoundationmodels/types.py (3)
GenerationResponse(192-285)text(229-243)StreamChunk(289-310)
applefoundationmodels/client.py (3)
applefoundationmodels/base_client.py (4)
BaseClient(17-151)_validate_platform(28-54)_initialize_library(57-65)_build_session_config(117-151)applefoundationmodels/async_client.py (2)
close(50-58)create_session(60-97)applefoundationmodels/_foundationmodels.pyi (1)
create_session(16-16)
applefoundationmodels/session.py (4)
applefoundationmodels/base_session.py (7)
BaseSession(20-214)_check_closed(42-50)_normalize_generation_params(52-65)_begin_generation(67-74)_build_generation_response(112-157)_end_generation(76-83)transcript(160-183)applefoundationmodels/types.py (5)
GenerationParams(81-98)NormalizedGenerationParams(102-138)GenerationResponse(192-285)StreamChunk(289-310)text(229-243)applefoundationmodels/pydantic_compat.py (1)
normalize_schema(116-144)applefoundationmodels/_foundationmodels.pyi (2)
generate(19-19)generate_structured(22-27)
🪛 GitHub Actions: Tests
applefoundationmodels/async_client.py
[error] 50-50: Return type "Coroutine[Any, Any, None]" of "close" incompatible with return type "None" in supertype "applefoundationmodels.base.ContextManagedResource" [override]
applefoundationmodels/__init__.py
[error] 61-61: ImportError: cannot import name '_foundationmodels' from partially initialized module 'applefoundationmodels' (likely due to a circular import) during execution of the command: python -c "import applefoundationmodels as afm; status = afm.Client.check_availability()..."
applefoundationmodels/async_session.py
[error] 56-56: Return type "Coroutine[Any, Any, None]" of "close" incompatible with return type "None" in supertype "applefoundationmodels.base.ContextManagedResource" [override]
applefoundationmodels/client.py
[error] 11-11: ImportError: cannot import name '_foundationmodels' from partially initialized module 'applefoundationmodels' (circular import detected) during module import. This occurs while executing: from . import _foundationmodels
🪛 Ruff (0.14.4)
tests/test_session.py
38-38: Unused method argument: check_availability
(ARG002)
67-67: Unused method argument: check_availability
(ARG002)
78-78: Unused method argument: check_availability
(ARG002)
126-126: Unused method argument: check_availability
(ARG002)
232-232: Unused method argument: check_availability
(ARG002)
301-301: Unused method argument: check_availability
(ARG002)
358-358: Unused method argument: check_availability
(ARG002)
applefoundationmodels/base_client.py
37-40: Avoid specifying long messages outside the exception class
(TRY003)
48-51: Avoid specifying long messages outside the exception class
(TRY003)
applefoundationmodels/types.py
240-242: Avoid specifying long messages outside the exception class
(TRY003)
257-259: Avoid specifying long messages outside the exception class
(TRY003)
applefoundationmodels/base_session.py
50-50: Avoid specifying long messages outside the exception class
(TRY003)
applefoundationmodels/async_session.py
144-146: Avoid specifying long messages outside the exception class
(TRY003)
217-217: Do not catch blind exception: Exception
(BLE001)
tests/conftest.py
61-63: Prefer TypeError exception for invalid type
(TRY004)
61-63: Avoid specifying long messages outside the exception class
(TRY003)
91-93: Avoid specifying long messages outside the exception class
(TRY003)
applefoundationmodels/session.py
142-144: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (13)
applefoundationmodels/async_session.py (5)
93-156: Well-structured unified generate API.The method correctly implements the unified generation interface with proper parameter validation and routing logic. The overloads provide good type safety for different use cases (text, structured, streaming).
158-174: LGTM - Proper async text generation implementation.The method correctly wraps the synchronous FFI call with
asyncio.to_threadand maintains transcript boundaries with proper error handling.
176-197: LGTM - Proper async structured generation implementation.The method correctly normalizes the schema, wraps the synchronous FFI call, and maintains proper transcript boundaries.
217-218: Broad exception catching is acceptable here.While static analysis flags the broad
Exceptioncatch, it's necessary here to propagate any error from the background thread to the main async context via the queue. The exception is re-raised on line 235, so this pattern is acceptable.
252-276: LGTM - Proper async history management.Both methods correctly wrap synchronous FFI calls with
asyncio.to_threadand maintain proper state management. Theclear_historymethod appropriately resets the transcript length tracking.README.md (8)
56-82: LGTM - Clear basic usage example.The example correctly demonstrates the Client API, session creation, and the new
response.textproperty access pattern. The code is accurate and matches the implementation.
84-103: LGTM - Excellent async streaming example.The async example correctly demonstrates
AsyncClient,await create_session(), and async streaming withasync for chunk in session.generate(..., stream=True). The usage pattern matches the AsyncSession implementation.
105-120: LGTM - Clear sync vs async comparison.The side-by-side comparison effectively illustrates the API symmetry between sync and async patterns. This helps users understand that the only difference is adding
async/awaitkeywords.
122-148: LGTM - Accurate structured output documentation.The example correctly shows the unified
generate()method with theschemaparameter and demonstrates accessing structured responses viaresponse.parsed. This matches the implementation in both Session and AsyncSession.
150-179: LGTM - Comprehensive Pydantic integration example.The example demonstrates all three ways to work with Pydantic models: passing the model as schema, accessing via
response.parsed, and using the convenienceresponse.parse_as()method. All patterns are accurate.
181-257: LGTM - Thorough tool calling documentation.The tool calling section is comprehensive and accurate, covering:
- Tool registration at session creation
- Accessing
response.tool_callsandresponse.finish_reason- Parsing tool call arguments
- Viewing full transcript with tool interactions
All examples match the implementation's API.
354-467: LGTM - Complete and accurate API reference.The API reference clearly documents both sync (
Client/Session) and async (AsyncClient/AsyncSession) interfaces, showing their symmetry. The unifiedgenerate()method signature with overloads for schema and stream parameters is accurately documented.
469-490: LGTM - Clear response type documentation.The
GenerationResponseandStreamChunktype documentation accurately describes the properties and methods available. The distinction between.textfor text responses and.parsedfor structured responses is well explained.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
applefoundationmodels/async_client.py(1 hunks)applefoundationmodels/async_session.py(1 hunks)applefoundationmodels/base.py(2 hunks)applefoundationmodels/base_client.py(1 hunks)applefoundationmodels/base_session.py(1 hunks)applefoundationmodels/client.py(4 hunks)applefoundationmodels/session.py(5 hunks)applefoundationmodels/tools.py(1 hunks)applefoundationmodels/types.py(2 hunks)tests/test_integration.py(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
applefoundationmodels/base.py (2)
applefoundationmodels/async_client.py (1)
aclose(76-90)applefoundationmodels/async_session.py (1)
aclose(94-106)
applefoundationmodels/async_client.py (5)
applefoundationmodels/base_client.py (3)
BaseClient(20-213)_session_class(47-53)_create_session_impl(154-178)applefoundationmodels/base.py (3)
AsyncContextManagedResource(40-65)close(31-37)aclose(58-65)applefoundationmodels/async_session.py (3)
AsyncSession(41-338)close(72-92)aclose(94-106)applefoundationmodels/client.py (3)
_session_class(43-45)close(47-55)create_session(57-87)applefoundationmodels/_foundationmodels.pyi (1)
create_session(16-16)
applefoundationmodels/async_session.py (5)
applefoundationmodels/base_session.py (10)
BaseSession(21-246)_call_ffi(44-59)_check_closed(61-69)_validate_generate_params(228-246)_get_temperature(71-73)_get_max_tokens(75-77)_begin_generation(79-86)_build_generation_response(124-169)_end_generation(88-95)transcript(172-195)applefoundationmodels/base.py (3)
AsyncContextManagedResource(40-65)close(31-37)aclose(58-65)applefoundationmodels/types.py (3)
GenerationResponse(152-245)StreamChunk(249-270)text(189-203)applefoundationmodels/pydantic_compat.py (1)
normalize_schema(116-144)applefoundationmodels/_foundationmodels.pyi (5)
generate_stream(30-35)generate(19-19)generate_structured(22-27)get_history(38-38)clear_history(39-39)
applefoundationmodels/client.py (2)
applefoundationmodels/base_client.py (3)
BaseClient(20-213)_session_class(47-53)_create_session_impl(154-178)applefoundationmodels/async_client.py (3)
_session_class(50-52)close(54-74)create_session(92-125)
applefoundationmodels/base_client.py (7)
applefoundationmodels/base.py (1)
ContextManagedResource(14-37)applefoundationmodels/types.py (1)
Availability(52-64)applefoundationmodels/exceptions.py (1)
NotAvailableError(26-29)applefoundationmodels/async_client.py (2)
_session_class(50-52)create_session(92-125)applefoundationmodels/client.py (2)
_session_class(43-45)create_session(57-87)applefoundationmodels/_foundationmodels.pyi (7)
init(6-6)check_availability(11-11)get_availability_reason(12-12)is_ready(13-13)get_version(8-8)create_session(16-16)register_tools(42-42)applefoundationmodels/tools.py (1)
register_tool_for_function(236-260)
applefoundationmodels/session.py (4)
applefoundationmodels/base_session.py (10)
BaseSession(21-246)_call_ffi(44-59)_check_closed(61-69)_validate_generate_params(228-246)_get_temperature(71-73)_get_max_tokens(75-77)_begin_generation(79-86)_build_generation_response(124-169)_end_generation(88-95)transcript(172-195)applefoundationmodels/types.py (3)
GenerationResponse(152-245)StreamChunk(249-270)text(189-203)applefoundationmodels/pydantic_compat.py (1)
normalize_schema(116-144)applefoundationmodels/_foundationmodels.pyi (2)
generate(19-19)generate_structured(22-27)
tests/test_integration.py (4)
applefoundationmodels/types.py (1)
text(189-203)tests/conftest.py (4)
assert_valid_response(41-68)session(30-35)client(22-26)assert_valid_chunks(71-96)applefoundationmodels/async_session.py (5)
generate(110-117)generate(121-128)generate(132-139)generate(141-202)aclose(94-106)applefoundationmodels/async_client.py (3)
AsyncClient(15-125)create_session(92-125)aclose(76-90)
applefoundationmodels/base_session.py (5)
applefoundationmodels/base.py (1)
ContextManagedResource(14-37)applefoundationmodels/types.py (4)
GenerationResponse(152-245)StreamChunk(249-270)ToolCall(124-148)Function(102-120)applefoundationmodels/async_session.py (1)
_call_ffi(59-70)applefoundationmodels/session.py (1)
_call_ffi(55-57)applefoundationmodels/_foundationmodels.pyi (1)
get_transcript(43-43)
🪛 GitHub Actions: Tests
applefoundationmodels/async_client.py
[error] 125-125: mypy: Incompatible return value type (got "Session | AsyncSession", expected "AsyncSession") [return-value]
applefoundationmodels/client.py
[error] 12-12: During import: from .session import Session; circular import detected leading to partially initialized module error.
applefoundationmodels/session.py
[error] 28-28: ImportError: cannot import name '_foundationmodels' from partially initialized module 'applefoundationmodels' (circular import).
🪛 Ruff (0.14.4)
applefoundationmodels/async_client.py
66-66: Local variable loop is assigned to but never used
Remove assignment to unused variable loop
(F841)
69-71: Abstract raise to an inner function
(TRY301)
69-71: Avoid specifying long messages outside the exception class
(TRY003)
applefoundationmodels/async_session.py
84-84: Local variable loop is assigned to but never used
Remove assignment to unused variable loop
(F841)
87-89: Abstract raise to an inner function
(TRY301)
87-89: Avoid specifying long messages outside the exception class
(TRY003)
264-264: Do not catch blind exception: Exception
(BLE001)
applefoundationmodels/base_client.py
65-68: Avoid specifying long messages outside the exception class
(TRY003)
76-79: Avoid specifying long messages outside the exception class
(TRY003)
applefoundationmodels/session.py
208-208: Do not catch blind exception: Exception
(BLE001)
applefoundationmodels/base_session.py
69-69: Avoid specifying long messages outside the exception class
(TRY003)
244-246: Avoid specifying long messages outside the exception class
(TRY003)
applefoundationmodels/types.py
200-202: Avoid specifying long messages outside the exception class
(TRY003)
217-219: Avoid specifying long messages outside the exception class
(TRY003)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
applefoundationmodels/async_session.py (1)
81-92: Fix the control flow in close() to prevent calling asyncio.run inside a running loop.The current try/except logic catches the
RuntimeErroryou raise for async callers and still executesasyncio.run(self.aclose()). When a loop is running,get_running_loop()succeeds (no exception), so you raise the instructional error—but thatRuntimeErroris immediately caught by yourexcept RuntimeErrorblock, which then callsasyncio.run()inside the active loop, triggering a different nested-loop error.Apply this diff to fix the logic:
- try: - # Check if we're in an async context with a running event loop - loop = asyncio.get_running_loop() - # If we get here, there's a running loop - we can't use asyncio.run() - # User should call aclose() instead in async contexts - raise RuntimeError( - "close() called from async context. Use 'await session.aclose()' instead." - ) - except RuntimeError: - # No running loop, safe to use asyncio.run() - asyncio.run(self.aclose()) + try: + asyncio.get_running_loop() + except RuntimeError: + # No running loop, safe to use asyncio.run() + asyncio.run(self.aclose()) + else: + # Running loop detected - user must call aclose() instead + raise RuntimeError( + "close() called from async context. Use 'await session.aclose()' instead." + )applefoundationmodels/async_client.py (1)
64-75: Fix the control flow in close() to prevent calling asyncio.run inside a running loop.The current try/except logic catches the
RuntimeErroryou raise for async callers and still executesasyncio.run(self.aclose()). When a loop is running,get_running_loop()succeeds (no exception), so you raise the instructional error—but thatRuntimeErroris immediately caught by yourexcept RuntimeErrorblock, which then callsasyncio.run()inside the active loop, triggering a different nested-loop error.Apply this diff to fix the logic:
- try: - # Check if we're in an async context with a running event loop - loop = asyncio.get_running_loop() - # If we get here, there's a running loop - we can't use asyncio.run() - # User should call aclose() instead in async contexts - raise RuntimeError( - "close() called from async context. Use 'await client.aclose()' instead." - ) - except RuntimeError: - # No running loop, safe to use asyncio.run() - asyncio.run(self.aclose()) + try: + asyncio.get_running_loop() + except RuntimeError: + # No running loop, safe to use asyncio.run() + asyncio.run(self.aclose()) + else: + # Running loop detected - user must call aclose() instead + raise RuntimeError( + "close() called from async context. Use 'await client.aclose()' instead." + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
applefoundationmodels/async_client.py(1 hunks)applefoundationmodels/async_session.py(1 hunks)applefoundationmodels/client.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
applefoundationmodels/client.py (2)
applefoundationmodels/base_client.py (3)
BaseClient(20-213)_session_class(47-53)_create_session_impl(154-178)applefoundationmodels/async_client.py (2)
_session_class(50-52)create_session(93-126)
applefoundationmodels/async_client.py (4)
applefoundationmodels/base_client.py (3)
BaseClient(20-213)_session_class(47-53)_create_session_impl(154-178)applefoundationmodels/base.py (3)
AsyncContextManagedResource(40-65)close(31-37)aclose(58-65)applefoundationmodels/async_session.py (3)
AsyncSession(40-337)close(71-92)aclose(94-106)applefoundationmodels/client.py (3)
_session_class(45-49)close(51-59)create_session(61-93)
🪛 GitHub Actions: Tests
applefoundationmodels/async_session.py
[error] 25-25: from . import _foundationmodels caused circular import during module initialization
applefoundationmodels/async_client.py
[error] 12-12: ImportError cascading from circular import: AsyncClient import depends on AsyncSession
🪛 Ruff (0.14.4)
applefoundationmodels/async_session.py
83-83: Local variable loop is assigned to but never used
Remove assignment to unused variable loop
(F841)
90-92: Avoid specifying long messages outside the exception class
(TRY003)
268-268: Do not catch blind exception: Exception
(BLE001)
applefoundationmodels/async_client.py
66-66: Local variable loop is assigned to but never used
Remove assignment to unused variable loop
(F841)
73-75: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (10)
applefoundationmodels/client.py (6)
8-14: Circular import issue successfully resolved.The TYPE_CHECKING guard and lazy imports correctly address the critical runtime failure flagged in previous reviews. Session is now only imported at type-checking time at the module level, with runtime imports deferred to method bodies.
31-42: LGTM!Proper delegation to BaseClient for initialization. The docstring accurately reflects the initialization behavior and exceptions raised by the parent class.
44-49: LGTM!Correctly implements the abstract property from BaseClient with lazy import to avoid circular dependencies.
51-59: LGTM!Proper resource cleanup implementation. Correctly iterates through sessions initialized by BaseClient and performs cleanup.
61-93: LGTM!Correctly delegates to BaseClient._create_session_impl and appropriately casts the result. The lazy import at line 91 is necessary for the cast operation and doesn't introduce circular dependencies since it's at runtime. The tools parameter is well-documented with a clear example.
96-113: LGTM!Clean context manager implementation with proper resource cleanup in the finally block.
applefoundationmodels/async_client.py (1)
77-91: LGTM: Clean async cleanup implementation.The
aclose()method correctly awaits each session's async cleanup and clears the tracking list.applefoundationmodels/async_session.py (3)
58-69: LGTM: Correct FFI routing for streaming vs non-streaming.The distinction between streaming (callback-based, runs directly) and non-streaming (wrapped in
asyncio.to_thread) is appropriate for handling the different threading models.
108-202: LGTM: Well-designed unified generate API with proper type overloads.The three-mode dispatch (text, structured, streaming) with type-safe overloads provides a clean, flexible interface. Parameter validation and default handling are correct.
246-309: LGTM: Robust async streaming implementation with proper exception propagation.The queue-based coordination with
run_coroutine_threadsafecorrectly bridges the background thread and async context. Exception handling at line 268 is intentional—it catches exceptions from the FFI layer and propagates them through the queue (re-raised at line 281), not a blind catch. The 5-second join timeout with warning (lines 298-307) prevents resource leaks while handling edge cases gracefully.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
applefoundationmodels/async_client.py(1 hunks)applefoundationmodels/async_session.py(1 hunks)applefoundationmodels/base.py(2 hunks)applefoundationmodels/base_client.py(1 hunks)applefoundationmodels/base_session.py(1 hunks)applefoundationmodels/session.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- applefoundationmodels/async_session.py
🧰 Additional context used
🧬 Code graph analysis (5)
applefoundationmodels/async_client.py (5)
applefoundationmodels/base_client.py (3)
BaseClient(20-213)_session_class(47-53)_create_session_impl(154-178)applefoundationmodels/base.py (2)
AsyncContextManagedResource(41-96)aclose(89-96)applefoundationmodels/async_session.py (2)
AsyncSession(45-301)aclose(102-114)applefoundationmodels/client.py (2)
_session_class(45-49)create_session(61-93)applefoundationmodels/_foundationmodels.pyi (1)
create_session(16-16)
applefoundationmodels/base_client.py (7)
applefoundationmodels/base.py (1)
ContextManagedResource(15-38)applefoundationmodels/types.py (1)
Availability(52-64)applefoundationmodels/exceptions.py (1)
NotAvailableError(26-29)applefoundationmodels/async_client.py (2)
_session_class(52-56)create_session(74-109)applefoundationmodels/client.py (2)
_session_class(45-49)create_session(61-93)applefoundationmodels/_foundationmodels.pyi (7)
init(6-6)check_availability(11-11)get_availability_reason(12-12)is_ready(13-13)get_version(8-8)create_session(16-16)register_tools(42-42)applefoundationmodels/tools.py (1)
register_tool_for_function(236-260)
applefoundationmodels/base_session.py (5)
applefoundationmodels/base.py (1)
ContextManagedResource(15-38)applefoundationmodels/types.py (4)
GenerationResponse(152-245)StreamChunk(249-270)ToolCall(124-148)Function(102-120)applefoundationmodels/async_session.py (4)
_call_ffi(63-75)_create_stream_queue(77-79)_create_stream_callback(81-86)_get_from_stream_queue(88-100)applefoundationmodels/session.py (4)
_call_ffi(55-57)_create_stream_queue(59-61)_create_stream_callback(63-65)_get_from_stream_queue(67-73)applefoundationmodels/_foundationmodels.pyi (2)
generate_stream(30-35)get_transcript(43-43)
applefoundationmodels/session.py (1)
applefoundationmodels/base_session.py (13)
BaseSession(26-441)_call_ffi(53-68)_create_stream_queue(167-174)_create_stream_callback(177-187)_get_from_stream_queue(190-203)_check_closed(70-78)_validate_generate_params(423-441)_apply_defaults(80-96)_generation_context(117-139)_build_generation_response(319-364)_begin_generation(98-105)_stream_chunks_impl(205-290)transcript(367-390)
applefoundationmodels/base.py (4)
applefoundationmodels/async_client.py (1)
aclose(58-72)applefoundationmodels/async_session.py (1)
aclose(102-114)applefoundationmodels/session.py (1)
close(75-77)applefoundationmodels/client.py (1)
close(51-59)
🪛 GitHub Actions: Tests
applefoundationmodels/async_client.py
[error] 71-71: Item "Session" of "Session | AsyncSession" has no attribute "aclose"
applefoundationmodels/base_client.py
[error] 43-43: mypy: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs
applefoundationmodels/base_session.py
[error] 141-142: Argument 1 to "asynccontextmanager" has incompatible type "Callable[[BaseSession], Generator[int, None, None]]"; expected "Callable[[BaseSession], AsyncIterator[Never]]"
[error] 142-142: The return type of an async generator function should be "AsyncGenerator" or one of its supertypes
applefoundationmodels/session.py
[error] 28-28: ImportError: cannot import name '_foundationmodels' from partially initialized module 'applefoundationmodels' (likely due to a circular import) during 'from . import _foundationmodels'.
🪛 Ruff (0.14.4)
applefoundationmodels/base_client.py
65-68: Avoid specifying long messages outside the exception class
(TRY003)
76-79: Avoid specifying long messages outside the exception class
(TRY003)
applefoundationmodels/base_session.py
36-36: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
78-78: Avoid specifying long messages outside the exception class
(TRY003)
240-240: Do not catch blind exception: Exception
(BLE001)
439-441: Avoid specifying long messages outside the exception class
(TRY003)
applefoundationmodels/base.py
76-76: Local variable loop is assigned to but never used
Remove assignment to unused variable loop
(F841)
83-86: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (21)
applefoundationmodels/base.py (2)
7-12: LGTM! Clean async infrastructure additions.The asyncio import and AT TypeVar are well-placed to support the new async context manager pattern.
41-96: Excellent async/sync lifecycle bridge implementation.The AsyncContextManagedResource class cleanly handles both async context manager protocol and sync-to-async bridging. The close() method correctly detects running event loops and provides helpful error guidance when misused.
The static analysis hints about the unused
loopvariable and long error message are false positives—the loop detection is intentional for its side effect, and the detailed error message provides valuable user guidance.applefoundationmodels/async_client.py (4)
7-14: Clean circular import resolution.The TYPE_CHECKING guard and deferred imports properly avoid circular dependencies while maintaining type safety.
38-49: LGTM! Clean initialization.The constructor properly delegates to BaseClient for shared initialization logic.
51-56: Good use of deferred import.The inline import in
_session_classproperly avoids circular dependencies while providing the correct session type.
74-109: Well-documented async session creation.The method correctly uses
cast()to narrow the return type from the base implementation and provides comprehensive documentation with examples.applefoundationmodels/base_client.py (4)
20-53: Excellent base class design.The BaseClient properly centralizes shared logic for sync/async clients. The abstract
_session_classproperty allows subclasses to specify their session type while reusing all initialization logic.
55-95: Robust platform validation and initialization.The platform checks ensure Apple Intelligence requirements (macOS Darwin 26.0+) with helpful error messages, and the one-time initialization pattern with the class-level
_initializedflag is correct.
97-152: Good API design for availability checking.The static methods allow users to check availability without instantiating a client, which is a clean UX pattern.
154-213: Clean session creation with proper tool registration.The
_create_session_impland_build_session_configmethods properly separate concerns—session creation uses the abstract_session_classpattern, and tool registration happens during config building.applefoundationmodels/session.py (4)
42-77: Clean Session implementation of base abstracts.The Session class correctly implements the abstract FFI and streaming methods from BaseSession, properly handling synchronous execution patterns.
79-183: Excellent unified generate() API design.The type overloads provide precise type hints for different usage modes (text, structured, streaming), and the implementation correctly delegates to specialized internal methods. The documentation is comprehensive with clear examples.
185-223: Well-structured generation implementations.The internal
_generate_*_implmethods properly use base class helpers (_generation_context,_build_generation_response,_stream_chunks_impl) to avoid code duplication.
225-249: Good history management.The
get_historyandclear_historymethods are straightforward, and the transcript length reset on clear is correct.applefoundationmodels/base_session.py (7)
26-96: Excellent base class design with proper initialization.The BaseSession properly centralizes shared logic for sync/async sessions. The initialization captures the transcript length to exclude initial instructions, which is a thoughtful detail for tracking generation boundaries.
116-140: Clean synchronous generation context manager.The
_generation_contextproperly handles generation boundaries with automatic cleanup on exceptions.
166-203: Good streaming abstraction design.The abstract methods for queue and callback creation allow Session and AsyncSession to provide sync/async implementations while sharing the core streaming logic.
205-291: Robust shared streaming implementation.The
_stream_chunks_implmethod properly handles:
- Background thread execution
- Exception propagation through the queue
- Proper thread cleanup with timeout
- Clear end-of-stream signaling
The warning log for threads that don't finish is a good defensive measure.
292-364: Clean tool call extraction and response building.The
_extract_tool_calls_from_transcriptand_build_generation_responsemethods correctly:
- Parse tool calls from transcript entries
- Build ToolCall/Function structures following OpenAI's pattern
- Set appropriate finish_reason based on tool calls
- Handle both text and structured generation
366-421: Well-designed transcript access pattern.The
transcriptproperty provides full history, whilelast_generation_transcriptuses slice indexing to return only the most recent generation. This is a clean API for inspecting generation-specific entries.
423-441: Appropriate parameter validation.The check preventing streaming with structured output is correct—this combination isn't supported and the error message clearly explains the constraint.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
applefoundationmodels/async_client.py(1 hunks)applefoundationmodels/async_session.py(1 hunks)applefoundationmodels/base_session.py(1 hunks)applefoundationmodels/session.py(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
applefoundationmodels/async_client.py (4)
applefoundationmodels/base_client.py (3)
BaseClient(20-213)_session_class(47-53)_create_session_impl(154-178)applefoundationmodels/base.py (4)
AsyncContextManagedResource(41-96)close(32-38)close(58-86)aclose(89-96)applefoundationmodels/async_session.py (3)
AsyncSession(45-310)close(102-109)aclose(111-123)applefoundationmodels/client.py (3)
_session_class(45-49)close(51-59)create_session(61-93)
applefoundationmodels/async_session.py (3)
applefoundationmodels/base_session.py (14)
BaseSession(36-454)_call_ffi(63-78)_create_stream_queue(177-184)_create_stream_callback(187-197)_get_from_stream_queue(200-213)_check_closed(80-88)_validate_generate_params(436-454)_apply_defaults(90-106)_async_generation_context(152-174)_build_generation_response(330-375)_begin_generation(108-115)_stream_chunks_impl(215-301)_end_generation(117-124)transcript(378-403)applefoundationmodels/base.py (4)
AsyncContextManagedResource(41-96)close(32-38)close(58-86)aclose(89-96)applefoundationmodels/session.py (12)
_call_ffi(54-56)_create_stream_queue(58-60)_create_stream_callback(62-64)_get_from_stream_queue(66-72)close(74-76)generate(91-98)generate(102-109)generate(113-120)generate(122-182)_generate_stream_impl(211-226)_generate_structured_impl(194-209)_generate_text_impl(184-192)
applefoundationmodels/session.py (2)
applefoundationmodels/base_session.py (13)
BaseSession(36-454)_call_ffi(63-78)_create_stream_queue(177-184)_create_stream_callback(187-197)_get_from_stream_queue(200-213)_check_closed(80-88)_validate_generate_params(436-454)_apply_defaults(90-106)_generation_context(127-149)_build_generation_response(330-375)_begin_generation(108-115)_stream_chunks_impl(215-301)transcript(378-403)applefoundationmodels/async_session.py (13)
_call_ffi(63-75)_create_stream_queue(77-79)_create_stream_callback(81-86)_get_from_stream_queue(88-100)close(102-109)generate(138-145)generate(149-156)generate(160-167)generate(169-229)_generate_stream_impl(265-282)_generate_structured_impl(245-263)_generate_text_impl(231-243)clear_history(299-308)
applefoundationmodels/base_session.py (5)
applefoundationmodels/base.py (1)
ContextManagedResource(15-38)applefoundationmodels/types.py (4)
GenerationResponse(152-245)StreamChunk(249-270)ToolCall(124-148)Function(102-120)applefoundationmodels/async_session.py (4)
_call_ffi(63-75)_create_stream_queue(77-79)_create_stream_callback(81-86)_get_from_stream_queue(88-100)applefoundationmodels/session.py (4)
_call_ffi(54-56)_create_stream_queue(58-60)_create_stream_callback(62-64)_get_from_stream_queue(66-72)applefoundationmodels/_foundationmodels.pyi (2)
generate_stream(30-35)get_transcript(43-43)
🪛 Ruff (0.14.4)
applefoundationmodels/base_session.py
88-88: Avoid specifying long messages outside the exception class
(TRY003)
251-251: Do not catch blind exception: Exception
(BLE001)
452-454: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests on macOS (3.10)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/test_integration.py (2)
36-48: AllowNonefromget_availability_reason().
Session.get_availability_reason()may returnNone; insisting on a non-empty string causes this test to fail on valid setups. Please acceptNoneand only check content when a string is provided.- reason = applefoundationmodels.Session.get_availability_reason() + reason = applefoundationmodels.Session.get_availability_reason() is_ready = applefoundationmodels.Session.is_ready() @@ - assert isinstance(reason, str), "Reason should be a string" + assert reason is None or isinstance(reason, str), "Reason should be optional string" @@ - assert reason, "Reason should not be empty" + if isinstance(reason, str): + assert reason, "Reason should not be empty when provided"
273-296: Use the validated text instead of slicing/len onGenerationResponse.
session.generate()returns aGenerationResponse; slicing it or callinglen()raisesTypeError. Capture the string returned fromassert_valid_response()(or use.text) before printing/truncating.- response = session.generate("") - # If it doesn't raise an error, check the response - assert_valid_response(response) - print(f"Response: {response[:50]}...") + response = session.generate("") + response_text = assert_valid_response(response) + # If it doesn't raise an error, check the response + print(f"Response: {response_text[:50]}...") @@ - response = session.generate(long_prompt[:500]) # Limit to reasonable size - assert_valid_response(response) - print(f"✓ Long prompt handled ({len(response)} chars in response)") + response = session.generate(long_prompt[:500]) # Limit to reasonable size + response_text = assert_valid_response(response) + print(f"✓ Long prompt handled ({len(response_text)} chars in response)")
🧹 Nitpick comments (1)
applefoundationmodels/base.py (1)
7-7: Drop the unusedasyncioimport.
asyncioisn’t referenced in this module anymore, so the import will trigger lint failures (e.g., Ruff F401). Removing it keeps the file clean.-import asyncio from abc import ABC, abstractmethod
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
README.md(10 hunks)applefoundationmodels/__init__.py(4 hunks)applefoundationmodels/async_session.py(1 hunks)applefoundationmodels/base.py(2 hunks)applefoundationmodels/base_session.py(1 hunks)applefoundationmodels/client.py(0 hunks)applefoundationmodels/session.py(4 hunks)tests/conftest.py(2 hunks)tests/test_client.py(0 hunks)tests/test_integration.py(5 hunks)tests/test_session.py(17 hunks)tests/test_session_lifecycle.py(1 hunks)tests/test_tools.py(6 hunks)
💤 Files with no reviewable changes (2)
- tests/test_client.py
- applefoundationmodels/client.py
🧰 Additional context used
🧬 Code graph analysis (10)
tests/test_session_lifecycle.py (5)
applefoundationmodels/types.py (1)
Availability(52-64)applefoundationmodels/session.py (2)
Session(41-270)close(85-87)applefoundationmodels/base_session.py (4)
check_availability(561-576)get_availability_reason(579-587)is_ready(590-597)get_version(600-607)tests/conftest.py (2)
check_availability(10-18)session(22-27)tests/test_session.py (3)
TestSessionLifecycle(117-133)test_session_context_manager(120-125)test_session_close(127-133)
tests/test_session.py (5)
applefoundationmodels/session.py (5)
Session(41-270)generate(102-109)generate(113-120)generate(124-131)generate(133-193)applefoundationmodels/types.py (3)
text(189-203)parsed(206-220)parse_as(222-245)tests/conftest.py (2)
session(22-27)check_availability(10-18)applefoundationmodels/base_session.py (3)
check_availability(561-576)transcript(401-424)last_generation_transcript(427-455)tests/test_session_lifecycle.py (2)
test_session_context_manager(49-54)test_session_close(56-63)
applefoundationmodels/base.py (2)
applefoundationmodels/async_session.py (1)
close(97-106)applefoundationmodels/session.py (1)
close(85-87)
tests/conftest.py (4)
applefoundationmodels/session.py (1)
Session(41-270)applefoundationmodels/base_session.py (2)
check_availability(561-576)get_availability_reason(579-587)applefoundationmodels/types.py (4)
Availability(52-64)GenerationResponse(152-245)text(189-203)StreamChunk(249-270)tests/test_tools.py (1)
session(78-83)
applefoundationmodels/session.py (2)
applefoundationmodels/base_session.py (14)
BaseSession(48-607)_call_ffi(89-104)_create_stream_queue(203-210)_create_stream_callback(213-223)_get_from_stream_queue(226-239)_mark_closed(477-484)_check_closed(106-114)_validate_generate_params(457-475)_apply_defaults(116-132)_generation_context(153-175)_build_generation_response(353-398)_begin_generation(134-141)_stream_chunks_impl(241-324)transcript(401-424)applefoundationmodels/types.py (3)
GenerationResponse(152-245)StreamChunk(249-270)text(189-203)
tests/test_integration.py (5)
applefoundationmodels/types.py (2)
Availability(52-64)text(189-203)tests/conftest.py (4)
assert_valid_response(33-60)assert_valid_chunks(63-88)check_availability(10-18)session(22-27)applefoundationmodels/session.py (6)
Session(41-270)generate(102-109)generate(113-120)generate(124-131)generate(133-193)close(85-87)applefoundationmodels/base_session.py (4)
check_availability(561-576)get_availability_reason(579-587)is_ready(590-597)get_version(600-607)applefoundationmodels/async_session.py (6)
generate(121-128)generate(132-139)generate(143-150)generate(152-212)AsyncSession(38-347)close(97-106)
tests/test_tools.py (4)
applefoundationmodels/session.py (5)
Session(41-270)generate(102-109)generate(113-120)generate(124-131)generate(133-193)tests/conftest.py (2)
session(22-27)check_availability(10-18)applefoundationmodels/base_session.py (3)
check_availability(561-576)transcript(401-424)is_ready(590-597)applefoundationmodels/types.py (1)
text(189-203)
applefoundationmodels/__init__.py (3)
applefoundationmodels/session.py (1)
Session(41-270)applefoundationmodels/async_session.py (1)
AsyncSession(38-347)applefoundationmodels/types.py (2)
GenerationResponse(152-245)StreamChunk(249-270)
applefoundationmodels/async_session.py (2)
applefoundationmodels/base_session.py (13)
BaseSession(48-607)_call_ffi(89-104)_create_stream_queue(203-210)_create_stream_callback(213-223)_get_from_stream_queue(226-239)_check_closed(106-114)_apply_defaults(116-132)_async_generation_context(178-200)_build_generation_response(353-398)_begin_generation(134-141)_stream_chunks_impl(241-324)_end_generation(143-150)transcript(401-424)applefoundationmodels/base.py (3)
AsyncContextManagedResource(41-65)close(32-38)close(58-65)
applefoundationmodels/base_session.py (7)
applefoundationmodels/base.py (1)
ContextManagedResource(15-38)applefoundationmodels/types.py (5)
GenerationResponse(152-245)StreamChunk(249-270)ToolCall(124-148)Function(102-120)Availability(52-64)applefoundationmodels/exceptions.py (1)
NotAvailableError(26-29)applefoundationmodels/_foundationmodels.pyi (9)
create_session(16-16)generate_stream(30-35)get_transcript(43-43)init(6-6)register_tools(42-42)check_availability(11-11)get_availability_reason(12-12)is_ready(13-13)get_version(8-8)applefoundationmodels/async_session.py (4)
_call_ffi(68-70)_create_stream_queue(72-74)_create_stream_callback(76-81)_get_from_stream_queue(83-95)applefoundationmodels/session.py (4)
_call_ffi(65-67)_create_stream_queue(69-71)_create_stream_callback(73-75)_get_from_stream_queue(77-83)applefoundationmodels/tools.py (1)
register_tool_for_function(236-260)
🪛 Ruff (0.14.4)
tests/test_session_lifecycle.py
49-49: Unused method argument: check_availability
(ARG002)
56-56: Unused method argument: check_availability
(ARG002)
65-65: Unused method argument: check_availability
(ARG002)
86-86: Unused method argument: check_availability
(ARG002)
92-92: Unused method argument: check_availability
(ARG002)
99-99: Unused method argument: check_availability
(ARG002)
tests/test_session.py
39-39: Unused method argument: check_availability
(ARG002)
68-68: Unused method argument: check_availability
(ARG002)
79-79: Unused method argument: check_availability
(ARG002)
120-120: Unused method argument: check_availability
(ARG002)
127-127: Unused method argument: check_availability
(ARG002)
233-233: Unused method argument: check_availability
(ARG002)
302-302: Unused method argument: check_availability
(ARG002)
359-359: Unused method argument: check_availability
(ARG002)
384-384: Unused method argument: check_availability
(ARG002)
tests/conftest.py
22-22: Unused function argument: check_availability
(ARG001)
53-55: Prefer TypeError exception for invalid type
(TRY004)
53-55: Avoid specifying long messages outside the exception class
(TRY003)
83-85: Avoid specifying long messages outside the exception class
(TRY003)
tests/test_tools.py
89-89: Unused method argument: check_availability
(ARG002)
104-104: Unused method argument: check_availability
(ARG002)
119-119: Unused method argument: check_availability
(ARG002)
143-143: Unused method argument: check_availability
(ARG002)
171-171: Unused method argument: check_availability
(ARG002)
205-205: Unused method argument: check_availability
(ARG002)
226-226: Unused method argument: check_availability
(ARG002)
248-248: Unused method argument: check_availability
(ARG002)
286-286: Unused method argument: check_availability
(ARG002)
301-301: Unused method argument: check_availability
(ARG002)
337-337: Unused method argument: check_availability
(ARG002)
366-366: Unused method argument: check_availability
(ARG002)
389-389: Unused method argument: check_availability
(ARG002)
415-415: Unused method argument: check_availability
(ARG002)
applefoundationmodels/async_session.py
258-260: Do not assign a lambda expression, use a def
Rewrite callback as a def
(E731)
279-279: Do not catch blind exception: Exception
(BLE001)
314-314: Do not catch blind exception: Exception
(BLE001)
315-315: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
applefoundationmodels/base_session.py
114-114: Avoid specifying long messages outside the exception class
(TRY003)
274-274: Do not catch blind exception: Exception
(BLE001)
473-475: Avoid specifying long messages outside the exception class
(TRY003)
496-499: Avoid specifying long messages outside the exception class
(TRY003)
507-510: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (32)
tests/test_session.py (6)
8-8: LGTM: Clean import of Session class.The direct import of
Sessionalongside the module import provides convenient access for test fixtures and direct instantiation.
15-63: LGTM: Tests correctly use response.text property.The tests properly access text responses via the
.textproperty onGenerationResponseobjects, aligning with the new unified API.Note: The static analysis warnings about unused
check_availabilityare false positives—pytest fixtures can provide setup/validation without being referenced in the function body.
68-88: LGTM: Streaming tests align with new StreamChunk API.The tests correctly iterate over the streaming response and validate
chunk.contentattributes, matching the newIterator[StreamChunk]return type.
136-194: LGTM: Structured output tests demonstrate proper API usage.The tests correctly:
- Use
response.parsedto access structured dictionaries- Demonstrate the
response.parse_as(Person)convenience method for Pydantic models- Validate both dict-based and Pydantic-based structured outputs
This provides good test coverage for the new structured output API surface.
196-358: LGTM: Comprehensive transcript tracking validation.The tests thoroughly validate:
- Empty transcript before generation
- Transcript accumulation across multiple calls
- Correct slicing of
last_generation_transcriptfrom full transcript- Transcript behavior after
clear_history()- Entry format and structure validation
The slice comparison logic
full_transcript[-len(last_transcript):] == last_transcriptcorrectly validates that recent entries match.
384-476: LGTM: Tool calling transcript validation is thorough.The test properly:
- Registers tools via
Session(tools=[...])- Validates tool_call and tool_output entries in transcripts
- Verifies tool call structure including JSON-encoded arguments
- Checks that
last_generation_transcriptcorrectly isolates each generation's entries- Explicitly closes the session (appropriate since not using context manager)
This provides strong coverage of the tool calling and transcript integration.
tests/test_tools.py (5)
14-75: LGTM: ToolTestHarness refactor improves testability.The updated design:
- Removes session coupling from
__init__- Provides
wrap_tool()for explicit tool wrapping with call tracking- Preserves function metadata (
__name__,__annotations__,__doc__) needed for schema extraction- Maintains internal tools list for reference
This is a cleaner, more flexible testing pattern that aligns with the new Session-based API.
86-268: LGTM: Tool registration tests align with new Session API.Tests correctly demonstrate:
- Direct tool registration via
Session(tools=[...])- Mix of wrapped (with harness) and unwrapped tools as appropriate
- Validation of tool invocation via response content
- Coverage of various parameter types (no params, single param, multiple params, optional params, mixed types)
The testing approach provides good coverage of the tool registration surface.
270-361: LGTM: Comprehensive tool_calls property validation.This new test class thoroughly validates:
tool_callsisNonewhen no tools registered or not invokedtool_callsis populated when tools are actually calledfinish_reasoncorrectly reflects "stop" vs "tool_calls"- ToolCall structure including
id,type, and nestedfunctionwithnameandarguments- Arguments are JSON-encoded strings (validates with
json.loads())Excellent coverage of the new tool call metadata surface on
GenerationResponse.
363-460: LGTM: Transcript tests validate tool interaction tracking.The tests properly verify:
- Tool call entries appear in transcript with correct type
- Required fields present (
tool_id,tool_name,argumentsfor calls;tool_id,contentfor outputs)- Arguments are JSON-encoded
- Content contains expected results
This ensures the transcript correctly captures the full tool interaction lifecycle.
462-532: LGTM: Integration tests validate real-world scenarios.The tests effectively:
- Use
Session.is_ready()for appropriate test skipping- Validate end-to-end tool execution
- Test large output handling (20KB) with start/end markers to verify no truncation
- Use context managers for proper cleanup
The large output test is particularly valuable for ensuring buffer resizing works correctly.
applefoundationmodels/async_session.py (6)
68-96: LGTM: Async FFI and queue implementations are sound.The implementations correctly:
- Use
asyncio.to_thread()to execute sync FFI calls asynchronously- Create
asyncio.Queuefor async coordination- Use
asyncio.run_coroutine_threadsafe()in callback to safely bridge threads- Poll the async queue from sync context in
_get_from_stream_queue()(needed since it's called from the sync generator_stream_chunks_impl)The
time.sleep(0.01)on Line 94 is appropriate here—this method is called from a sync context and needs to poll the async queue without blocking the event loop indefinitely.
97-106: LGTM: Async close implementation is correct.The
close()method properly implements theAsyncContextManagedResourceprotocol by marking the session as closed. The async signature aligns with the base class requirements.
108-212: LGTM: Generate method properly implements async patterns.The implementation correctly:
- Provides type overloads for text, structured, and streaming modes
- Returns
Coroutine[Any, Any, GenerationResponse]for awaitable operations- Returns
AsyncIterator[StreamChunk]for async streaming (used withasync for)- Delegates to internal implementation methods
- Includes comprehensive docstring with examples
The unified
generate()surface successfully mirrors the syncSessionAPI while respecting async semantics.
214-244: LGTM: Text and structured generation implementations are correct.Both methods properly:
- Use
async with self._async_generation_context()for boundary tracking- Await FFI calls via
self._call_ffi()- Return properly constructed
GenerationResponseobjectsThe async patterns correctly mirror the sync implementations while maintaining event loop cooperation.
246-319: LGTM: Async streaming implementation bridges sync generator to async correctly.The implementation effectively:
- Spawns the sync generator
_stream_chunks_implin a background thread viaThreadPoolExecutor- Uses
asyncio.run_coroutine_threadsafe()to safely forward chunks to the event loop- Awaits
output_queue.get()to yield chunks without blocking the event loop- Handles exceptions by forwarding them through the queue
- Uses sentinel value
_DONEto signal completion- Properly cleans up executor and calls
_end_generation()Regarding static analysis warnings:
- The lambda on Line 258 is acceptable—it's a simple callback that needs to capture
loop- Broad exception catching (Lines 279, 314) is appropriate for thread boundary error handling
logging.error()on Line 315 is fine—the error context is clear from the messageThis is a sophisticated but sound approach to providing async streaming while reusing the shared sync generator implementation from
BaseSession.
321-345: LGTM: Async history methods are implemented correctly.Both methods properly:
- Use async signatures
- Await FFI calls via
self._call_ffi()clear_history()correctly resets_last_transcript_lengthto maintain transcript boundary trackingSimple and correct async implementations.
applefoundationmodels/session.py (4)
41-88: LGTM: Sync Session implementations are clean and correct.The implementations properly:
- Execute FFI calls synchronously in
_call_ffi()- Use
queue.Queuefor synchronous streaming coordination- Poll queue with timeout to handle blocking appropriately
- Implement simple
close()that marks session closedAll abstract methods from
BaseSessionare correctly implemented for synchronous operation.
89-193: LGTM: Unified generate method provides clean API surface.The implementation successfully:
- Defines clear type overloads for text, structured, and streaming modes
- Returns appropriate types (
GenerationResponseorIterator[StreamChunk])- Validates parameters before dispatching
- Applies defaults via
_apply_defaults()- Delegates to focused internal implementation methods
- Includes comprehensive documentation with examples
This unified interface significantly improves API ergonomics compared to separate methods.
195-242: LGTM: Internal generation implementations follow consistent patterns.All three methods correctly:
- Use appropriate context managers for lifecycle tracking
- Call FFI functions via
_call_ffi()- Build responses via
_build_generation_response()- Handle streaming by yielding from shared
_stream_chunks_impl()- Ensure cleanup via try/finally in streaming path
The consistent patterns across generation modes make the code maintainable and predictable.
244-268: LGTM: History methods are simple and correct.Both methods:
- Check for closed state appropriately
- Use
_call_ffi()for FFI accessclear_history()correctly updates_last_transcript_lengthto maintain boundary tracking after clearingClean sync implementations.
README.md (3)
54-306: LGTM: Examples comprehensively demonstrate the new API.The documentation effectively:
- Shows
Session/AsyncSessionusage with context managers- Demonstrates
response.textfor text andresponse.parsedfor structured outputs- Illustrates streaming with both sync and async patterns
- Provides extensive tool calling examples including:
- Basic tool registration via
Session(tools=[...])- Accessing
tool_callsandfinish_reasonon responses- Schema extraction from docstrings and type hints
- Transcript viewing for tool interactions
- Includes Pydantic integration with
response.parse_as()The examples are clear, practical, and cover the major features well.
350-472: LGTM: API reference accurately documents the public interface.The documentation correctly describes:
SessionandAsyncSessionwith identical signatures (justawaitdifference)- Unified
generate()method with schema and stream parametersGenerationResponsewith.text,.parsed, and.parse_as()membersStreamChunkstructure- Properties like
transcriptandlast_generation_transcript- Static utility methods
This provides clear reference documentation for the updated API surface.
556-660: LGTM: Architecture and auxiliary documentation are consistent.The documentation:
- Updates architecture diagram to reflect
session.py/async_session.pysplit- Accurately describes the project structure with new files
- Uses current API in troubleshooting examples (
Session.get_availability_reason())- Maintains consistency across all sections
The documentation updates comprehensively support the API refactoring.
applefoundationmodels/base_session.py (8)
40-46: LGTM: Cached module accessor prevents circular imports.The
@lru_cachedecoratedget_foundationmodels()effectively:
- Defers the
_foundationmodelsimport until first use- Caches the module to avoid repeated imports
- Breaks the circular dependency that occurred with module-level imports
This pattern was correctly applied based on past review feedback.
48-115: LGTM: Base session initialization and core abstractions are well-designed.The implementation:
- Validates platform requirements before proceeding
- Initializes the library once (class-level flag)
- Creates FFI instance and session with proper configuration
- Tracks closed state and transcript boundaries
- Defines clear abstract interface (
_call_ffi) for subclassesThe
_check_closed(),_apply_defaults(), and other helper methods provide clean abstractions for shared logic.Note: Static analysis warning about message length on Line 114 can be ignored—the error message is concise and appropriate.
134-201: LGTM: Generation context managers cleanly manage lifecycle.Both context managers properly:
- Mark generation start via
_begin_generation()- Yield start_length for response building
- Ensure
_end_generation()is called in exception handlers- Use correct types (
Generatorfor sync,AsyncGeneratorfor async)This provides consistent boundary tracking across all generation modes with proper cleanup guarantees.
202-325: LGTM: Shared streaming implementation effectively bridges sync and async.The design:
- Defines clear abstracts for queue/callback creation (implemented differently by sync/async subclasses)
- Runs
generate_streamin background thread to avoid blocking- Yields
StreamChunkobjects with properfinish_reasonand indexing- Forwards exceptions through the queue for proper error handling
- Waits up to 5 seconds for thread cleanup and logs warnings if incomplete
This shared implementation successfully serves both
SessionandAsyncSessionwith their respective queue types.Note: The broad exception catch on Line 274 (BLE001 warning) is appropriate—it's catching exceptions at a thread boundary where we need to forward any error to the consumer.
326-399: LGTM: Tool extraction and response building are well-structured.The methods effectively:
- Extract tool calls from transcript entries into
ToolCallobjects with nestedFunctiondata- Centralize response building to avoid duplication
- Set
finish_reasoncorrectly ("tool_calls" when tools used, "stop" otherwise)- Handle structured generation specially (no tool calls, always "stop")
- Update transcript boundaries via
_end_generation()This provides consistent response handling across all generation modes.
400-456: LGTM: Transcript properties provide clean access to conversation history.Both properties correctly:
- Check for closed state before accessing
- Use type casts for static analysis clarity
last_generation_transcriptproperly slices from_last_transcript_lengthto isolate recent entries- Include comprehensive docstrings with examples
The slicing approach efficiently provides access to both full and recent transcript without duplication.
457-485: LGTM: Parameter validation and lifecycle methods are straightforward.The methods correctly:
- Validate that streaming and structured output aren't combined (unsupported)
- Provide
_mark_closed()for both sync and async close implementationsSimple, focused helpers that enforce constraints appropriately.
Note: Static analysis TRY003 warnings about message length are not concerning—the error messages are clear and appropriately detailed.
486-607: LGTM: Static methods provide essential platform validation and utilities.The implementations correctly:
- Validate macOS platform and version requirements (≥26.0)
- Initialize library once using class-level flag
- Build session config and register tools via
tools.register_tool_for_function()- Expose useful static utilities (
check_availability(),is_ready(),get_version())- Provide detailed error messages for platform failures
These foundational methods ensure proper initialization and provide convenient static access to availability checks.
Note: Static analysis TRY003 warnings about long error messages can be ignored—detailed error messages are valuable for troubleshooting platform issues.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/conftest.py(2 hunks)tests/test_tools.py(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/test_tools.py (4)
applefoundationmodels/session.py (5)
Session(41-270)generate(102-109)generate(113-120)generate(124-131)generate(133-193)tests/conftest.py (2)
session(20-25)check_availability(10-16)applefoundationmodels/base_session.py (3)
check_availability(561-576)transcript(401-424)is_ready(590-597)applefoundationmodels/types.py (1)
text(189-203)
tests/conftest.py (4)
applefoundationmodels/base_session.py (2)
check_availability(561-576)get_availability_reason(579-587)applefoundationmodels/session.py (1)
Session(41-270)applefoundationmodels/types.py (4)
Availability(52-64)GenerationResponse(152-245)text(189-203)StreamChunk(249-270)tests/test_tools.py (1)
session(78-83)
🪛 Ruff (0.14.4)
tests/test_tools.py
78-78: Unused function argument: check_availability
(ARG001)
89-89: Unused method argument: check_availability
(ARG002)
104-104: Unused method argument: check_availability
(ARG002)
119-119: Unused method argument: check_availability
(ARG002)
143-143: Unused method argument: check_availability
(ARG002)
171-171: Unused method argument: check_availability
(ARG002)
205-205: Unused method argument: check_availability
(ARG002)
226-226: Unused method argument: check_availability
(ARG002)
248-248: Unused method argument: check_availability
(ARG002)
286-286: Unused method argument: check_availability
(ARG002)
301-301: Unused method argument: check_availability
(ARG002)
337-337: Unused method argument: check_availability
(ARG002)
366-366: Unused method argument: check_availability
(ARG002)
389-389: Unused method argument: check_availability
(ARG002)
415-415: Unused method argument: check_availability
(ARG002)
466-466: Unused method argument: check_availability
(ARG002)
484-484: Unused method argument: check_availability
(ARG002)
tests/conftest.py
20-20: Unused function argument: check_availability
(ARG001)
51-53: Prefer TypeError exception for invalid type
(TRY004)
51-53: Avoid specifying long messages outside the exception class
(TRY003)
81-83: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests on macOS (3.10)
🔇 Additional comments (7)
tests/conftest.py (2)
9-16: LGTM! Appropriate scope and updated API usage.The session-scoped fixture correctly uses the new
Session.check_availability()andSession.get_availability_reason()static methods.
19-25: LGTM! Proper fixture dependency and cleanup.The
check_availabilityparameter creates a fixture dependency ensuring availability checks run before session creation. The static analysis warning (ARG001) is a false positive—this is standard pytest fixture dependency syntax.Good practice using
instructions=Noneto avoid transcript pollution.tests/test_tools.py (5)
17-54: LGTM! Well-implemented tool wrapping.The new
wrap_toolmethod correctly preserves function metadata using both@wrapsdecorator and explicit attribute copying. The call capture mechanism is clean and the implementation aligns well with the new Session-based architecture.
77-83: LGTM! Proper fixture dependency and Session usage.The
check_availabilityparameter establishes a fixture dependency. The static analysis warning is a false positive—this is standard pytest practice.
89-117: LGTM! Correct migration to Session API.Tests properly use
Session(tools=[...])and access responses via.textproperty, correctly aligning with the new API surface.
270-360: LGTM! Comprehensive testing of new tool_calls property.Excellent test coverage of the new
GenerationResponse.tool_callsproperty across multiple scenarios:
- No tools registered
- Tools registered but not called
- Tools called (with structure validation)
- Multiple tools called
The tests properly validate the data structures including JSON-encoded arguments.
462-532: LGTM! Excellent edge case testing.The integration tests properly use
Session.is_ready()and include a valuable edge case test for large tool outputs (20KB) to verify buffer resizing. The verification of both START and END markers ensures no truncation occurs.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
applefoundationmodels/async_session.py(1 hunks)applefoundationmodels/base_session.py(1 hunks)applefoundationmodels/session.py(3 hunks)applefoundationmodels/tools.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- applefoundationmodels/tools.py
🧰 Additional context used
🧬 Code graph analysis (3)
applefoundationmodels/session.py (4)
applefoundationmodels/base_session.py (11)
BaseSession(54-655)_call_ffi(107-122)_create_stream_queue_adapter(246-248)_StreamQueueAdapter(72-75)_mark_closed(525-532)_plan_generate_call(152-175)_generation_context(196-218)_build_generation_response(401-446)_begin_generation(177-184)_stream_chunks_impl(250-262)transcript(449-472)applefoundationmodels/types.py (3)
GenerationResponse(152-245)StreamChunk(249-270)text(189-203)applefoundationmodels/pydantic_compat.py (1)
normalize_schema(116-144)applefoundationmodels/async_session.py (9)
_call_ffi(66-68)_create_stream_queue_adapter(70-90)push(75-76)get_sync(81-84)close(92-101)generate(116-123)generate(127-134)generate(138-145)generate(147-202)
applefoundationmodels/async_session.py (5)
applefoundationmodels/base_session.py (12)
BaseSession(54-655)_call_ffi(107-122)_create_stream_queue_adapter(246-248)_StreamQueueAdapter(72-75)_plan_generate_call(152-175)_async_generation_context(221-243)_build_generation_response(401-446)_begin_generation(177-184)_stream_chunks_async_impl(264-280)_end_generation(186-193)_check_closed(124-132)transcript(449-472)applefoundationmodels/base.py (3)
AsyncContextManagedResource(41-65)close(32-38)close(58-65)applefoundationmodels/types.py (3)
GenerationResponse(152-245)StreamChunk(249-270)text(189-203)applefoundationmodels/pydantic_compat.py (1)
normalize_schema(116-144)applefoundationmodels/session.py (14)
_call_ffi(64-66)_create_stream_queue_adapter(68-82)push(72-73)get_sync(75-80)close(84-86)generate(101-108)generate(112-119)generate(123-130)generate(132-190)_generate_stream_impl(224-236)_generate_structured_impl(205-222)_generate_text_impl(192-203)get_history(238-251)clear_history(253-262)
applefoundationmodels/base_session.py (7)
applefoundationmodels/base.py (1)
ContextManagedResource(15-38)applefoundationmodels/types.py (5)
GenerationResponse(152-245)StreamChunk(249-270)ToolCall(124-148)Function(102-120)Availability(52-64)applefoundationmodels/exceptions.py (1)
NotAvailableError(26-29)applefoundationmodels/async_session.py (5)
push(75-76)get_sync(81-84)get_async(78-79)_call_ffi(66-68)_create_stream_queue_adapter(70-90)applefoundationmodels/session.py (4)
push(72-73)get_sync(75-80)_call_ffi(64-66)_create_stream_queue_adapter(68-82)applefoundationmodels/_foundationmodels.pyi (9)
create_session(16-16)generate_stream(30-35)get_transcript(43-43)init(6-6)register_tools(42-42)check_availability(11-11)get_availability_reason(12-12)is_ready(13-13)get_version(8-8)applefoundationmodels/tools.py (1)
register_tool_for_function(230-250)
🪛 GitHub Actions: Tests
applefoundationmodels/session.py
[error] 251-251: Returning Any from function declared to return "list[Any]" [no-any-return]
applefoundationmodels/async_session.py
[error] 92-92: Return type "Coroutine[Any, Any, None]" of "close" incompatible with return type "None" in supertype "applefoundationmodels.base.ContextManagedResource" [override]
[error] 263-263: Returning Any from function declared to return "list[Any]" [no-any-return]
applefoundationmodels/base_session.py
[error] 635-635: Returning Any from function declared to return "str | None" [no-any-return]
[error] 645-645: Returning Any from function declared to return "bool" [no-any-return]
[error] 655-655: Returning Any from function declared to return "str" [no-any-return]
🪛 Ruff (0.14.4)
applefoundationmodels/async_session.py
82-84: Avoid specifying long messages outside the exception class
(TRY003)
applefoundationmodels/base_session.py
132-132: Avoid specifying long messages outside the exception class
(TRY003)
273-273: Avoid specifying long messages outside the exception class
(TRY003)
294-294: Do not catch blind exception: Exception
(BLE001)
521-523: Avoid specifying long messages outside the exception class
(TRY003)
544-547: Avoid specifying long messages outside the exception class
(TRY003)
555-558: Avoid specifying long messages outside the exception class
(TRY003)
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/tool_calling_comprehensive.py (1)
83-107: Restore availability guard before creating the session.
Session()raisesNotAvailableErrorwhen Apple Intelligence is disabled. Because thewith Session(...):block now sits outside any guard, the example will crash before entering thetry/exceptwhen run on unsupported machines. Please reinstate the availability check (e.g., callcheck_availability_or_exit()before creating the session) so the script exits cleanly instead of throwing.To fix, import the helper and bail out early:
-from applefoundationmodels import Session +from applefoundationmodels import Session +from utils import check_availability_or_exit @@ def main(): + if not check_availability_or_exit(verbose=False): + return 1 @@ - with Session( + with Session(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(7 hunks)examples/basic_async_chat.py(1 hunks)examples/basic_chat.py(2 hunks)examples/streaming_chat.py(4 hunks)examples/structured_output.py(5 hunks)examples/tool_calling_comprehensive.py(4 hunks)examples/utils.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
examples/structured_output.py (3)
applefoundationmodels/session.py (5)
Session(40-264)generate(101-108)generate(112-119)generate(123-130)generate(132-190)tests/conftest.py (1)
session(20-25)applefoundationmodels/types.py (2)
parsed(206-220)text(189-203)
examples/basic_async_chat.py (3)
applefoundationmodels/async_session.py (1)
AsyncSession(36-276)applefoundationmodels/exceptions.py (1)
NotAvailableError(26-29)examples/utils.py (2)
check_availability_or_exit(11-29)handle_example_error(45-58)
examples/streaming_chat.py (3)
applefoundationmodels/async_session.py (1)
AsyncSession(36-276)examples/utils.py (1)
check_availability_or_exit(11-29)applefoundationmodels/session.py (4)
generate(101-108)generate(112-119)generate(123-130)generate(132-190)
examples/basic_chat.py (5)
applefoundationmodels/session.py (1)
Session(40-264)applefoundationmodels/exceptions.py (1)
NotAvailableError(26-29)examples/utils.py (1)
check_availability_or_exit(11-29)applefoundationmodels/base_session.py (1)
get_version(648-655)applefoundationmodels/types.py (1)
text(189-203)
examples/utils.py (3)
applefoundationmodels/types.py (1)
Availability(52-64)applefoundationmodels/exceptions.py (1)
NotAvailableError(26-29)tests/conftest.py (1)
check_availability(10-16)
examples/tool_calling_comprehensive.py (2)
applefoundationmodels/session.py (1)
Session(40-264)applefoundationmodels/types.py (1)
text(189-203)
🪛 LanguageTool
README.md
[uncategorized] ~3-~3: The operating system from Apple is written “macOS”.
Context: ...ccess to the on-device LLM available in MacOS 26+. ## Features - Async Support:...
(MAC_OS)
🪛 Ruff (0.14.4)
examples/basic_async_chat.py
1-1: Shebang is present but file is not executable
(EXE001)
56-56: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (9)
examples/basic_chat.py (4)
1-13: LGTM! Clean migration to Session-based imports.The updated imports and documentation correctly reflect the new Session-based API.
18-28: Well-structured session initialization.The availability check and Session context manager usage correctly demonstrate the new API pattern.
40-43: Correct response handling.Properly uses
response.textproperty to access generated text fromGenerationResponse.
49-59: Proper history access and error handling.The example correctly demonstrates conversation history retrieval and appropriate exception handling for the new API.
README.md (5)
57-90: Clear and accurate Quick Start examples.The basic usage, streaming, and async/await examples correctly demonstrate the new Session and AsyncSession APIs with proper response handling patterns.
92-145: Well-documented structured output functionality.Examples correctly demonstrate both JSON schema and Pydantic model usage, including the
response.parsedproperty andresponse.parse_as()convenience method.
187-228: Comprehensive parameter and session management examples.The examples correctly demonstrate generation parameters and session lifecycle management including multiple sessions, history clearing, and cleanup.
331-352: Response type definitions look accurate.The
GenerationResponseandStreamChunktype definitions are clearly documented. Note that the earlier verification comment (lines 173-182) abouttool_callsandfinish_reasonapplies here as well.
173-182: The propertiesresponse.tool_callsandresponse.finish_reasonare correctly documented and implemented inGenerationResponse. The verification confirms both are defined as class attributes inapplefoundationmodels/types.py(lines 184-185), with proper documentation in the docstring (lines 162-163). The README code snippet is accurate and requires no changes.Likely an incorrect or invalid review comment.
Adopt a more consistent interface. This has breaking changes.
Summary by CodeRabbit
New Features
Removed
Other