fix(deriver): clear dangling session hint on pending dreams at session delete - #992
fix(deriver): clear dangling session hint on pending dreams at session delete#992Rajat-Ahuja1997 wants to merge 1 commit into
Conversation
…n delete A dream consolidates an entire (observer, observed) collection, not a single session — its payload.session_name is a config/context hint set to the session of the most recent explicit document (see DreamScheduler._execute_dream). Dream queue items therefore carry session_id=None and a work_unit_key with no session component, so neither delete in delete_session reaches them. That left pending dreams pointing at a deleted session, where run_dream's config resolution raises ResourceNotFoundException and the task errors out for no good reason. Strip the stale hint instead of dropping the dream, so it runs workspace-scoped — a state the orchestrator already supports (the manual dream route enqueues it whenever session_id is omitted). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WalkthroughSession deletion now removes ChangesSession deletion cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/crud/test_session.py (1)
200-209: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover processed records and JSON key removal.
Lines 202-209 treat a missing
session_namekey andsession_name: Noneas the same result. Add a processed dream withtest_session.nameand assert that it retains its hint. Assert that"session_name"is absent from the unprocessed target payload. This validates the required preservation of historical rows and the JSONB key-removal behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/crud/test_session.py` around lines 200 - 209, Extend the dream fixture/assertion coverage around the existing payload-comprehension assertion to include a processed dream referencing test_session.name and verify that its session hint remains unchanged. Separately assert that the unprocessed deleted-session dream’s payload does not contain the “session_name” key, distinguishing JSONB key removal from a present None value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/crud/test_session.py`:
- Around line 200-209: Extend the dream fixture/assertion coverage around the
existing payload-comprehension assertion to include a processed dream
referencing test_session.name and verify that its session hint remains
unchanged. Separately assert that the unprocessed deleted-session dream’s
payload does not contain the “session_name” key, distinguishing JSONB key
removal from a present None value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95e99290-3f98-49ca-8c3e-44813314a6e9
📒 Files selected for processing (2)
src/crud/session.pytests/crud/test_session.py
What
delete_sessionnow stripspayload.session_namefrom pending dream queue items that reference the session being deleted.Why
Surfaced while reviewing #801 / #799.
delete_sessionalready deletes the session'sQueueItemandActiveQueueSessionrows, but dreams slip through both:session_id=None(src/deriver/enqueue.py:441), so thesession_iddelete misses them.dream:{dream_type}:{workspace}:{observer}:{observed}— no session component — so thesplit_part(key, ':', 3) == session_namematch misses them too.A dream isn't scoped to a session in the first place: it consolidates the whole
(observer, observed)collection.payload.session_nameis only a config/context hint, set byDreamScheduler._execute_dreamto the session of the most recent explicit document for that pair (src/dreamer/dream_scheduler.py:195-205).So the stale row isn't stale work — it's a stale tag. Left alone,
run_dreamresolves that session for configuration (src/dreamer/orchestrator.py:112), raisesResourceNotFoundException, and the dream errors out for no good reason.How
Clear the hint rather than delete the dream, leaving it workspace-scoped — a state the orchestrator already handles (
session_name is None→ workspace config), and one the manual dream route produces wheneversession_idis omitted (src/routers/workspaces.py:232).Scoped to
processed = false; historical rows are left to the reconciler.Testing
tests/crud/test_session.py::TestSessionCRUD::test_delete_session_clears_dangling_dream_session_hint— three dreams (one on the deleted session, one on another session, one workspace-wide); asserts all three survive and only the first loses its hint. Fails without the fix.uv run pytest tests/crud/ tests/routes/test_sessions.py→ 116 passed.Summary by CodeRabbit
Bug Fixes
Tests