Skip to content

fix: drop rt.Session from public surface and error guidance - #1399

Closed
aryansk wants to merge 1 commit into
RailtownAI:mainfrom
aryansk:fix/1376-remove-rt-session-reference
Closed

aryansk wants to merge 1 commit into
RailtownAI:mainfrom
aryansk:fix/1376-remove-rt-session-reference

Conversation

@aryansk

@aryansk aryansk commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Problem

Closes #1376. When no runner is active, safe_get_runner_context() raises ContextError with notes that point users at rt.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: the ContextError notes now point at the public rt.Flow API instead of rt.Session().
  • __init__.py: Session and session are removed from __all__, so they no longer appear in the documented public surface (pdoc renders from __all__) or star-imports.
  • Both names remain reachable as module attributes (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

uv run python -m pytest packages/railtracks/tests/unit_tests/context   packages/railtracks/tests/unit_tests/prompt packages/railtracks/tests/unit_tests/test_pending_changes.py   packages/railtracks/tests/unit_tests/test_session.py packages/railtracks/tests/unit_tests/test_exceptions.py   packages/railtracks/tests/unit_tests/observability -q
result: 199 passed

uv run ruff check packages/railtracks/src/railtracks/ && uv run ruff format --check
result: All checks passed / 259 files already formatted

Full unit suite: 2008 passed; the 18 failures + 16 collection errors are
pre-existing on clean main (optional-extra deps: portkey, boto3, datasets).

Documentation and release impact

  • User-facing documentation updated (public __all__ surface)
  • Changelog/release note needed
  • Migration or compatibility note needed (names remain reachable, so none)
  • No documentation impact

Review notes

  • Known limitations: Session/session are still accessible as attributes for backward compatibility; a future release can remove them entirely.
  • Follow-up issue, if any: none
  • Security/licensing considerations: none

…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

@soulFood5632 soulFood5632 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@soulFood5632

Copy link
Copy Markdown
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.

@soulFood5632

Copy link
Copy Markdown
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

@Amir-R25

Amir-R25 commented Sep 9, 2026

Copy link
Copy Markdown
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!

@Amir-R25

Copy link
Copy Markdown
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!

@Amir-R25 Amir-R25 closed this Sep 17, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove active reference to rt.session

3 participants