Repository navigation
Conversation
…AI#1376) The ContextError raised when no runner is active pointed users at rt.Session(), which is largely internal. Point them at the public rt.Flow API instead, and remove Session/session from __all__ so they are no longer part of the documented public interface (they remain reachable as attributes for backward compatibility). Closes RailtownAI#1376
Collaborator
|
There is a minor conflict here that you will need to handle on your end but it looks good to me. Let me know once it is ready for review and I'll trigger the tests and we can merge. |
Collaborator
|
@aryansk pinging here to see if you would still like to handle the conflicts on your end. If not I would be happy to take over |
Collaborator
|
Hey @aryansk, just pinging this PR again since it's been in draft for a month, we'd close it in a few days if we don't hear back from you. Thanks for your interest in helping our project! |
Collaborator
|
@aryansk I'm closing this as we did not hear back from you, please do another PR if you still want to contribute. Thanks for your interest in Railtracks! |
6 of 9 tasks
CoronRing
added a commit
that referenced
this pull request
Sep 18, 2026
Session is largely internal, so the framework should stop steering users toward it. - The ContextError notes raised by safe_get_runner_context and get_publisher now show the rt.Flow entry point instead of `with rt.Session():`. - Session/session leave __all__, so pdoc and star-imports no longer advertise them. Both stay reachable as module attributes and in __dir__, the same shape `interactive` already has in this module, so existing code keeps working. Unlike `interactive`, they stay silent on access. - The MaxCalls budget-scoping note told users to wrap top-level calls in `with rt.Session():`; it now uses a single flow entry point, which is the pattern the rest of that page already uses. Also fills in the broken "(see __ for more details)" placeholder in the flow_name warning with the flows documentation link it promised. Carried over from #1399, which was closed for inactivity. Closes #1376 Co-authored-by: Aryan Singh K. <70511529+aryansk@users.noreply.github.com>
CoronRing
added a commit
that referenced
this pull request
Sep 22, 2026
## Summary Closes #1376. `Session` is largely internal, so the framework should stop steering users toward it. - `context/central.py`: the `ContextError` notes raised by `safe_get_runner_context` **and** `get_publisher` now show the `rt.Flow` entry point. The issue named both sites. - `__init__.py`: `Session` and `session` leave `__all__`, so pdoc (which renders from `__all__`) and star-imports no longer advertise them. They stay reachable as module attributes and in `__dir__` — the same shape `interactive` already has in this module, and the line the diff replaces said as much. Unlike `interactive`, they stay silent on access, since `test_core_entry_points_are_silent` requires `rt.Session` not to warn. - `_session.py`: the `flow_name` warning promised docs via a literal `(see __ for more details)`. It now carries the flows link. Raised in [this comment](#1376 (comment)). - `max_calls.md`: the budget-scoping note told users to wrap top-level calls in `with rt.Session():`. It now uses a single flow entry point, the pattern the rest of that page already uses. Removing the names outright would be a hard break (they are used widely in tests and by existing users). Dropping them from `__all__` keeps the advertised surface clean while leaving every existing import working. ## Credit This carries over @aryansk's work in #1399, which was closed for inactivity rather than for anything wrong with it — it had already been reviewed with "looks good to me" and only needed a conflict resolved. The approach here is theirs; they are credited as co-author on the commit. Thank you for the original patch. Rebased onto current `main` and extended: the second `ContextError` site the issue named, the `(see __ ...)` placeholder from the newer issue comment, and a `with rt.Session():` snippet that landed in the `MaxCalls` docs after that PR was opened. The `prompts/prompt.py` hunk from #1399 is dropped, since that function was rewritten upstream and no longer mentions `Session`. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Breaking change - [x] Docs - [ ] Refactor / chore / build / tests ## Checklist - [x] Lint & format pass (`ruff check . && ruff format .`) - [x] Tests added/updated and pass locally (`pytest tests`) - [x] Docs updated if user-facing behavior changed - [x] Breaking changes include migration notes (none needed: both names stay reachable) ## Notes Verified locally: - `pytest` unit + integration: 2812 passed, 10 skipped - `ruff check` and `ruff format --check`: clean - `mkdocs build --strict`: builds - `scripts/check_dependencies_sorted.py`: passes - pdoc output no longer lists `Session`/`session`, while `session_id` and the other members stay - `from railtracks import *` no longer binds `Session`; `rt.Session` still resolves and appears in `dir(rt)` The 18 test errors and 4 collection errors in a local full run are pre-existing on `main` (optional extras: `duckdb`, `tavily`, SQL-loader deps), as is the `docs/scripts/tools_mcp_guides.py:256` mypy error. The single test covers the one invariant that is not self-evident from the diff: the `from ._session import Session as Session` aliases are what keep the names importable once they are out of `__all__`, and that form reads like dead code, so a lint autofix or a tidy-up would silently break every `rt.Session` user. On the `MaxCalls` page, the warning box keeps its original "is its own run" title. The body no longer puts `Session` in front of the reader as the unit they reason about, so the title should not either. Adjacent finding, deliberately **not** fixed here: the `flow_name` warning whose text this PR touches also fires on railtracks' own internal call paths, so a bare top-level `rt.call()` warns about an API the user never touched. Pre-existing since #946; filed as #1574 rather than widened into this PR. Co-authored-by: Aryan Singh K. <70511529+aryansk@users.noreply.github.com>
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.
Problem
Closes #1376. When no runner is active,
safe_get_runner_context()raisesContextErrorwith notes that point users atrt.Session(). Session is largely internal, and the maintainer asked to stop actively referencing it and to remove it from the public interface.Change
context/central.py: theContextErrornotes now point at the publicrt.FlowAPI instead ofrt.Session().__init__.py:Sessionandsessionare removed from__all__, so they no longer appear in the documented public surface (pdoc renders from__all__) or star-imports.rt.Session,rt.session) for backward compatibility; only the advertised interface changes.prompts/prompt.py: internal comment updated to match.Why this approach
Removing the names entirely would be a hard breaking change (they are used widely in tests and by existing users). Keeping them importable while dropping them from
__all__matches the repository's own precedent for de-emphasized public names and the maintainer's explicit request.Testing
Documentation and release impact
__all__surface)Review notes
Session/sessionare still accessible as attributes for backward compatibility; a future release can remove them entirely.