Skip to content

perf: avoid per-message refreshes when renaming sessions - #15926

Open
olegsuhoparov wants to merge 1 commit into
langflow-ai:release-1.14.0from
olegsuhoparov:fix/session-rename-refresh
Open

olegsuhoparov wants to merge 1 commit into
langflow-ai:release-1.14.0from
olegsuhoparov:fix/session-rename-refresh

Conversation

@olegsuhoparov

@olegsuhoparov olegsuhoparov commented Oct 22, 2026 •

Copy link
Copy Markdown

Summary

Renaming an ordinary or shared chat session currently refreshes every message after flushing the new session_id, adding one SELECT per message.

Build the existing MessageResponse objects directly from the flushed ORM rows. Both queries already load complete messages, and MessageTable has no update-generated columns. Ownership filters, flush/commit/rollback handling, and response fields remain unchanged.

Testing

  • Focused monitor suites: 51 passed (1 warning).
    LITELLM_LOCAL_MODEL_COST_MAP=True uv run --no-sync pytest \
      -p no:codeflash-benchmark -p no:rerunfailures \
      src/backend/tests/unit/api/v1/test_monitor_session_rename.py \
      src/backend/tests/unit/api/v1/test_monitor_shared.py \
      src/backend/tests/unit/api/v1/test_monitor_auth.py \
      src/backend/tests/unit/api/v1/test_monitor_ownership.py -q --disable-warnings
  • The four new query-count cases fail on unchanged upstream code: 2 message SELECTs for one message and 9 for eight messages, on both routes. With the fix, each request uses one message SELECT and returns the same complete responses.
  • Regression coverage includes persisted values, cross-user isolation, shared-flow isolation, missing sessions, authentication, and rollback after a write failure.
  • Changed-file Ruff formatting/lint and repository pre-commit checks pass, including secret detection.
  • The current make lint target reports that no type checker is configured.

Local tests use SQLite and the bundled LiteLLM cost map; benchmark and retry plugins are disabled. No dependencies or test configuration were changed.

Summary by CodeRabbit

  • Bug Fixes
    • Session renaming now returns updated message details while preserving other message information.
    • Renaming affects only messages belonging to the selected session. Requests for missing or inaccessible sessions continue to return an error without changing stored messages.
    • Failed updates do not persist partial changes.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 38f354b and 4606783.

📒 Files selected for processing: 2
  • src/backend/base/langflow/api/v1/monitor.py
  • src/backend/tests/unit/api/v1/test_monitor_session_rename.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The regular and shared-flow session-renaming endpoints now build response objects from flushed messages without refreshing each message. New tests cover response data, database changes, query count, access errors, and update failure.

Changes

Session Rename

Layer / File(s) Summary
Build responses from flushed messages
src/backend/base/langflow/api/v1/monitor.py, src/backend/tests/unit/api/v1/test_monitor_session_rename.py
Both rename endpoints validate responses from flushed ORM objects instead of refreshing each message. Tests check returned and stored message data, one message-table SELECT, missing or foreign sessions, unauthenticated requests, and update failure.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: cristhianzl

Merge Risk: ⚪ Minimal · up to 46067

No actionable merge-blocking risk was identified; the rename responses can be built without the per-message refreshes.

Pre-merge checks: Passed 8 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 8 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely states the main performance change: avoiding per-message refreshes during session renaming.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Test Coverage For New Implementations Passed The PR adds src/backend/tests/unit/api/v1/test_monitor_session_rename.py, which follows the backend test_*.py convention and directly covers both changed rename endpoints through the ordinary an…
Test Quality And Coverage Passed The PR adds focused async pytest coverage for both changed endpoints through the parametrized ordinary/shared route fixture. Success tests compare complete serialized MessageResponse values, verify on…
Test File Naming And Structure Passed The pull request adds src/backend/tests/unit/api/v1/test_monitor_session_rename.py, which matches the backend test_*.py pattern and the unit-test directory. It uses pytest fixtures, parametrizatio…
Excessive Mock Usage Warning Passed The new test file does not use mock objects, unittest.mock, patching, or stubs. It exercises the real API client, authentication, database session, ORM models, and SQLAlchemy engine. The event listene…
Full details: Test Coverage For New Implementations

Explanation

The PR adds src/backend/tests/unit/api/v1/test_monitor_session_rename.py, which follows the backend test_*.py convention and directly covers both changed rename endpoints through the ordinary and shared route parameters. The tests verify the new behavior with one message-table SELECT, complete response fields, persisted session changes, ownership isolation, missing sessions, authentication, and rollback on update failure. The tests use the project’s API client and database fixtures, so they are functional unit coverage rather than placeholders.

Full details: Test Quality And Coverage

Explanation

The PR adds focused async pytest coverage for both changed endpoints through the parametrized ordinary/shared route fixture. Success tests compare complete serialized MessageResponse values, verify only owned messages change, verify persisted state, and assert one message SELECT for one and eight messages. Error coverage includes missing and foreign sessions (404), unauthenticated requests (403), and injected update failures with no persisted changes (500). The tests use the repository's pytest and async client patterns and are behavioral tests, not smoke tests.

Full details: Test File Naming And Structure

Explanation

The pull request adds src/backend/tests/unit/api/v1/test_monitor_session_rename.py, which matches the backend test_*.py pattern and the unit-test directory. It uses pytest fixtures, parametrization, async tests supported by the repository's asyncio_mode = "auto", and context-manager/finally cleanup for event listeners. Test names describe the behavior under test. Coverage includes ordinary and shared routes, successful renames, query-count behavior, missing and foreign sessions, authentication, and rollback after a write failure. No frontend or integration-test change is present.

Full details: Excessive Mock Usage Warning

Explanation

The new test file does not use mock objects, unittest.mock, patching, or stubs. It exercises the real API client, authentication, database session, ORM models, and SQLAlchemy engine. The event listeners only count SQL statements and inject a database failure; they do not replace core logic. The tests therefore do not show excessive mock usage or a need for additional integration coverage under this check.

✨ Finishing Touches: 💡 1
🧪 Generate unit tests (beta)
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the performance Maintenance tasks and housekeeping label Oct 22, 2026
@olegsuhoparov
olegsuhoparov marked this pull request as ready for review October 22, 2026 13:33
@github-actions github-actions Bot added performance Maintenance tasks and housekeeping and removed performance Maintenance tasks and housekeeping labels Oct 22, 2026
@olegsuhoparov

Copy link
Copy Markdown
Author

Could a maintainer please rerun Python 3.14 Group 7 on head 460678325b7e757886e82ddb4bb743f2db4f4898?

The unchanged test_idle_live_stream_emits_comments_and_preserves_terminal_events[True-langflow] test hit its 2-second timeout in test_workflow_stream_keepalive.py:87. The downstream performance report then rejected the incomplete Group 7 timings. The session-rename regression in that shard passed.

My single rerun request returned HTTP 403 (Must have admin rights to Repository.), so I could not verify whether the timeout reproduces. Thank you.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Maintenance tasks and housekeeping

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant