Repository navigation
fix: make session runtime metadata and REPL model changes stick - #86
Merged
Merged
Conversation
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Contributor
📝 WalkthroughWalkthroughAdds REPL context hooks for clear and model-change handling, delegates Changes
Sequence DiagramsequenceDiagram
participant Client
participant REPL
participant Hook as on_model_change (async)
participant SessionMgr
participant Runtime
participant Agent
Client->>REPL: /model <new_model>
REPL->>Hook: await on_model_change(new_model)
Hook->>SessionMgr: replace_session_runtime_config(session_id, model_name)
SessionMgr->>Runtime: build/terminate/replace runtime (may reuse provider)
SessionMgr->>Agent: rebind adapter/pipeline/context
Agent->>Runtime: initialize or bind runtime
Runtime-->>Hook: ready / error
Hook-->>REPL: completion (success or rollback)
REPL-->>Client: report model switch result
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coding-agent/src/coding_agent/cli/repl.py`:
- Around line 275-276: The code sets managed_session.model_name then calls the
synchronous _session_manager._persist_session which can block the event loop;
replace this with an asynchronous persistence call (e.g., await
self._session_manager.persist_session or await
self._session_manager._persist_session_async(managed_session) if such helper
exists) or offload the blocking call to a thread executor (e.g.,
loop.run_in_executor) so the REPL /model path does not perform sync I/O on the
event loop; update the async model-change path where managed_session and
model_name are set to use the awaited async persistence method or
executor-wrapped call.
In `@coding-agent/tests/cli/test_repl.py`:
- Around line 652-655: The test currently skips asserting the /clear callback
when session._on_clear is missing because it uses getattr(session, "_on_clear",
None) and a callable() guard; change this to require the callback
unconditionally by either asserting hasattr(session, "_on_clear") first or by
accessing session._on_clear directly (e.g., on_clear = session._on_clear), then
call on_clear() and assert redraw_calls == ["redraw"] so the test fails if the
callback is absent or broken.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 549df86a-db52-45f9-a3cf-6c12bec42cc7
📒 Files selected for processing (8)
coding-agent/src/coding_agent/cli/commands.pycoding-agent/src/coding_agent/cli/repl.pycoding-agent/src/coding_agent/ui/http_server.pycoding-agent/src/coding_agent/ui/schemas.pycoding-agent/src/coding_agent/ui/session_manager.pycoding-agent/tests/cli/test_repl.pycoding-agent/tests/ui/test_http_server.pycoding-agent/tests/ui/test_session_manager_public_api.py
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Owner
Author
|
@codex[agent] Please review this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8b7907b32
ℹ️ 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".
Comment on lines
+278
to
+282
| await self._session_manager.shutdown_session_runtime(session_id) | ||
| managed_session = self._session_manager.get_session(session_id) | ||
| managed_session.model_name = model_name | ||
| await self._session_manager._persist_session_async(managed_session) | ||
| await self._switch_session(session_id) |
There was a problem hiding this comment.
The /model handler now shuts down the current runtime before verifying that the replacement runtime can be created. If _persist_session_async or _switch_session fails (for example, store I/O issues or agent initialization failures for the requested model), the previous adapter has already been closed and the REPL is left pointing at a torn-down runtime, so later turns can fail even though the command only reports a recoverable error. Defer teardown until after the new runtime is ready, or restore the old runtime on failure.
Useful? React with 👍 / 👎.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
coding-agent/tests/cli/test_repl.py (1)
551-560: 💤 Low valueConsider using the module-level
_make_confighelper.
TestFooterIntegration._make_configduplicates the module-level_make_config()function defined at lines 32-48. The implementations are nearly identical.♻️ Proposed fix
class TestFooterIntegration: - def _make_config(self) -> Config: - return Config( - model="gpt-4o", - api_key=None, - provider="openai", - repo=Path("."), - base_url=None, - max_steps=30, - approval_mode="auto", - ) - def test_init_creates_footer(self, monkeypatch): from coding_agent.cli.repl import InteractiveSession monkeypatch.setattr(InteractiveSession, "_setup_agent", lambda self: None) - session = InteractiveSession(self._make_config()) + session = InteractiveSession(_make_config()) assert hasattr(session, "_footer")Apply similar changes to other usages of
self._make_config()in this class.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coding-agent/tests/cli/test_repl.py` around lines 551 - 560, The TestFooterIntegration._make_config method duplicates the module-level _make_config helper; remove the class method and update all uses of self._make_config() inside TestFooterIntegration to call the module-level _make_config() instead (ensure the module-level function name _make_config is in scope); update any other usages in the class to use _make_config() so there is a single shared implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@coding-agent/tests/cli/test_repl.py`:
- Around line 551-560: The TestFooterIntegration._make_config method duplicates
the module-level _make_config helper; remove the class method and update all
uses of self._make_config() inside TestFooterIntegration to call the
module-level _make_config() instead (ensure the module-level function name
_make_config is in scope); update any other usages in the class to use
_make_config() so there is a single shared implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e79e77eb-9779-4969-a2e7-6a97e0bdeae4
📒 Files selected for processing (3)
coding-agent/src/coding_agent/cli/repl.pycoding-agent/src/coding_agent/ui/session_manager.pycoding-agent/tests/cli/test_repl.py
🚧 Files skipped from review as they are similar to previous changes (1)
- coding-agent/src/coding_agent/cli/repl.py
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
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.
Summary
/modelchanges so the next turn actually uses the new model.Verification
uv run pytest tests/cli/test_repl.py::TestReplInitialization::test_model_command_rebuilds_pipeline_on_next_turn tests/ui/test_http_server.py::TestSessionCreation::test_create_session_accepts_runtime_provider_metadata tests/ui/test_http_server.py::TestSessionCreation::test_create_session_rejects_invalid_runtime_provider tests/ui/test_session_manager_public_api.py::test_session_as_dict_includes_restart_metadata -quv run basedpyright src/coding_agent/cli/commands.py src/coding_agent/cli/repl.py src/coding_agent/ui/http_server.py src/coding_agent/ui/schemas.py src/coding_agent/ui/session_manager.py tests/cli/test_repl.py tests/ui/test_http_server.py tests/ui/test_session_manager_public_api.py --level erroruv run python -m compileall -q src testsSummary by CodeRabbit
New Features
Improvements
Tests