Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7465e00ede
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self._can_parallelize_collaborate(job): | ||
| _ensure_not_running_loop("run", "arun") | ||
| report, parallel_state = asyncio.run( | ||
| self._arun_collaborate_parallel( |
There was a problem hiding this comment.
Preserve sync adapter compatibility in collaborate run
This branch routes synchronous run() through _arun_collaborate_parallel, which in turn uses _arun_worker/worker.arun and therefore adapter.acomplete; as a result, sync collaborate runs now depend on async adapter methods. Any BaseModelAdapter implementation that only supports complete(...) (previously sufficient for sync entrypoints) will now fail in this common path, turning successful sync workloads into failures. Keep the existing sync worker path as a fallback when async completion is not implemented.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex In async worker execution, WorkerRunner explicitly falls back from adapter.acomplete(...) to adapter.complete(...) on NotImplementedError:
- src/blackgeorge/worker_runner.py:233
- src/blackgeorge/worker_runner.py:246
- Same fallback exists for streaming path:
- src/blackgeorge/worker_runner.py:289
- src/blackgeorge/worker_runner.py:302
false positive 99%
There was a problem hiding this comment.
Summary
- Added a new
SyncOnlyAdaptertest double that implements onlycomplete(...)(noacomplete(...)) to model backward-compatible sync-only adapters explicitly. tests/test_workforce.pyL182-L203 - Added a regression test
test_workforce_sync_parallel_supports_sync_only_adapterto verifyDesk.run()in collaborate-mode parallel execution still succeeds and uses the sync adapter path correctly. tests/test_workforce.pyL269-L281 - Committed the change as
test: add sync-only adapter coverage for collaborate parallel runand created a follow-up PR message via themake_prtool.
Testing
- ✅
uv run pytest tests/test_workforce.py::test_workforce_sync_parallel_supports_sync_only_adapter -v - ✅
uv run pytest tests/test_workforce.py::test_workforce_sync_parallel_drains_async_event_handlers tests/test_workforce.py::test_workforce_sync_parallel_supports_sync_only_adapter -v - ✅
uv run ruff check tests/test_workforce.py
Summary
This PR ships the 1.1.8 runtime refactor focused on faster execution paths, cleaner sync/async ergonomics, and stronger correctness around pause/resume and event delivery.
What changed
Why
Verification
Notes