docs: update Session Hierarchy page with concurrency-safety guarantees#478
docs: update Session Hierarchy page with concurrency-safety guarantees#478MervinPraison wants to merge 1 commit into
Conversation
fixes #477) - Add agent-centric Quick Start with Agent example first - Include hero Mermaid sequence diagram showing concurrent fork/message scenario - Add Concurrency & Safety section listing all thread-safe operations - Provide realistic user interaction flow example with threading - Use Mintlify components (Steps, AccordionGroup, CardGroup) - Update frontmatter with sidebarTitle and improved description - Add How It Works section with visual flow diagram - Follow AGENTS.md standards for progressive disclosure and user-friendly tone 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 12 minutes and 1 second. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Session Hierarchy documentation to include sequence diagrams, quick start guides, concurrency safety details, a threading example, and best practices. The review feedback identifies three key issues: first, the Mermaid sequence diagram uses unsupported class directives that will cause rendering errors; second, the concurrency safety claims are inaccurate because operations like fork_session do not hold the file lock for the entire read-modify-write cycle; and third, the provided threading example is unsafe and prone to race conditions where messages can be lost.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| classDef user fill:#8B0000,stroke:#7C90A0,color:#fff | ||
| classDef agent fill:#189AB4,stroke:#7C90A0,color:#fff | ||
| classDef store fill:#10B981,stroke:#7C90A0,color:#fff | ||
|
|
||
| class User user | ||
| class Agent,ForkWorker agent | ||
| class Store store |
There was a problem hiding this comment.
Mermaid sequence diagrams do not support classDef or class directives. Including them will cause rendering errors in Mintlify/Mermaid. Please remove these lines to ensure the diagram renders correctly.
| Every mutation goes through a locked read-modify-write, so chat messages added in one process are never lost when another process forks, snapshots, reverts, shares, or retitles the same session. | ||
|
|
||
| | Operation | Safe with concurrent `add_message`? | | ||
| |---|---| | ||
| | `create_session(parent_id=...)` | Yes | | ||
| | `fork_session()` | Yes | | ||
| | `create_snapshot()` | Yes | | ||
| | `revert_to_snapshot()` | Yes | | ||
| | `revert_to_message()` | Yes | | ||
| | `share_session()` / `unshare_session()` | Yes | | ||
| | `set_title()` | Yes | | ||
| | `add_message()` | Yes (#1745) | |
There was a problem hiding this comment.
The documentation states that operations like fork_session, create_snapshot, revert_to_snapshot, etc., are safe under concurrent add_message writes. However, looking at the implementation in praisonaiagents/session/hierarchy.py, these methods release the file lock between loading and saving the session (unlike add_message which uses _modify_session_locked to hold the lock for the entire read-modify-write cycle). This means concurrent writes can be overwritten and lost. Please update the documentation to accurately reflect this limitation.
Every mutation on HierarchicalSessionStore is not fully safe under concurrent writes. Only add_message uses a locked read-modify-write cycle that prevents data loss. Other operations like fork_session, create_snapshot, and revert_to_snapshot release the lock between loading and saving, which can lead to concurrent messages being overwritten and lost.
| Operation | Safe with concurrent add_message? |
|---|---|
| create_session(parent_id=...) | No |
| fork_session() | No |
| create_snapshot() | No |
| revert_to_snapshot() | No |
| revert_to_message() | No |
| share_session() / unshare_session() | No |
| set_title() | No |
| add_message() | Yes (#1745) |
| # Background job forks for experiment (concurrent with user) | ||
| import threading | ||
| def experiment(): | ||
| fork_id = store.fork_session(session_id, title="Weather Analysis") | ||
| store.add_message(fork_id, "user", "Analyze historical patterns") | ||
|
|
||
| thread = threading.Thread(target=experiment) | ||
| thread.start() | ||
|
|
||
| # User sends another message during the fork | ||
| store.add_message(session_id, "user", "What about tomorrow?") | ||
|
|
||
| thread.join() | ||
| # Both messages survive: original session has both weather messages, | ||
| # fork has the pre-fork conversation only |
There was a problem hiding this comment.
This concurrent threading example is actually unsafe and prone to a race condition where the user's message "What about tomorrow?" can be lost. Because fork_session does not hold the file lock during its entire read-modify-write cycle, the concurrent add_message can be overwritten when fork_session saves the parent session back to disk. Please add a warning or modify the example to avoid concurrent mutations on the same session.
1 participant
Summary
Updates docs/features/session-hierarchy.mdx with comprehensive concurrency safety documentation following PR #1790 in the main PraisonAI repository.
Key Changes
Concurrency Safety: Documents that all mutating methods on HierarchicalSessionStore now use locked read-modify-write operations, ensuring chat messages added concurrently are never lost during fork/snapshot/revert operations.
User-Focused: Follows AGENTS.md requirement to start with agent-centric examples and provide copy-paste runnable code.
Visual Learning: Includes Mermaid diagrams using standard color scheme to help users understand both the race condition scenario and the internal locking flow.
Fixes #477
Generated with Claude Code