fix(diffusion): inherit cpu offload hooks in from_pipe pipelines - #3
Open
ChristianGruend wants to merge 2 commits into
Open
fix(diffusion): inherit cpu offload hooks in from_pipe pipelines#3ChristianGruend wants to merge 2 commits into
ChristianGruend wants to merge 2 commits into
Conversation
from_pipe() doesn't carry over offload hooks, causing shared components to load fully onto GPU and OOM when --cpu-offload is set.
|
The following required sections are missing or incomplete. Please update the PR description to address them:
This comment is deleted automatically once all sections are complete. |
Author
|
Note: |
Z-Gamez
pushed a commit
to Z-Gamez/odysseus-roundtable
that referenced
this pull request
Jul 25, 2026
… (#267)
* docs: add implementation plan for fixing chat context drifting (#135)
* fix: make Session.history immutable + fix {}.history crash
- Session.history now exposes a COPY of the internal _history list
- add_message() replaces history with a fresh copy each time
- get_context_messages() derives from _history directly
- replace_messages() updates both _history and history
- truncate_messages() updates both _history and history
- _persist_message() line 207: fixed {}.history fallback crash
- Added 11 tests for session isolation and edge cases
Addresses #135 root cause arcahyadi#1: shared mutable references
* fix: task scheduler uses SessionManager methods instead of overwriting sessions
- Added ensure_task_session() to SessionManager (checks cache first)
- Task scheduler now uses ensure_task_session() instead of direct dict assignment
- Task scheduler now uses SessionManager.add_message() for message persistence
- Removed direct sess_obj.history.append() that was silently losing data
Addresses #135 root causes arcahyadi#2 and arcahyadi#3
* fix: add age guard to cleanup_empty_sessions — don't delete sessions <1h old
Prevents the cleanup task from deleting sessions that were just created
and haven't received any messages yet (message_count == 0).
Addresses #135 root cause arcahyadi#5
* test: comprehensive session isolation tests (10/10 passing)
* refactor: consolidate _session_manager into singleton pattern
- Added set_session_manager_instance / get_session_manager_instance to core/models
- kept backward-compat aliases (set_session_manager, get_session_manager)
- session_manager.py re-exports the singleton functions
- ai_interaction.set_session_manager now syncs with the core singleton
- context_compactor uses get_session_manager_instance() instead of getattr hack
- app.py initializes the singleton once
Addresses #135 root cause arcahyadi#4: fragile global wiring
* test: add concurrent session isolation integration tests
Verifies:
- Concurrent add_message to different sessions doesn't cross-contaminate
- Rapid parallel writes maintain isolation
- Read-write concurrent access is safe
All 3 async tests pass, proving the immutable history fix works under concurrency
* fix: pre-import core.models in conftest to prevent test pollution
test_agent_loop.py stubs sys.modules['core.models'] = MagicMock() at
module level during collection. Any test collected after it imports
Session as a MagicMock. Pre-importing core.models in conftest.py
before test_agent_loop.py's module-level code runs prevents this.
* fix: make .history authoritative mutable list, address PR review
Per review feedback: keep .history as the authoritative mutable list so
existing code doing .history.pop(), .history = [...], etc. still works.
Fix the cross-contamination bug by ensuring __post_init__() gives each
Session its OWN unique history list (never shared).
Changes:
- core/models.py: .history IS the authoritative list. _history aliases it.
Each Session gets its own list in __post_init__.
- core/session_manager.py: add_message() delegates to Session.add_message()
instead of appending directly — no double-append, single source of truth.
- tests/test_session_manager.py: updated test to reflect that .history
references see new messages (same list, not a snapshot).
- docs/plans/2026-06-01-fix-chat-context-drifting.md: removed (not for
shipping — useful design context but too much process/doc to ship).
All 272 tests pass (3 pre-existing failures unrelated).
* Fix session manager message persistence
* Fix session history alias regressions
* Fix session history aliasing and task delivery
Z-Gamez
pushed a commit
to Z-Gamez/odysseus-roundtable
that referenced
this pull request
Jul 25, 2026
fix(docker): bump Docker CLI to a patched release
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
1 participant
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
from_pipe()does not carry over CPU offload hooks from the source pipeline. When--cpu-offloadis enabled, inpaint and img2img pipelines created viafrom_pipe()load their shared components fully onto the GPU instead of respecting the offload setting, causing an out-of-memory crash.Target branch
main—devno longer exists in this repository as of the recent restructuring;mainis the only branch available.Linked Issue
Fixes # — N/A, Issues are disabled on this repository.
Type of Change
Checklist
devdocker compose up) and verified the change works end-to-end.How to Test
--cpu-offloadenabled.from_pipe()internally to share the base pipeline).enable_model_cpu_offload()is called explicitly and the pipeline runs without OOM.Visual / UI changes
Not applicable — this is a backend-only fix in
scripts/diffusion_server.py, no UI/rendering code touched.