galaxyproject / galaxyproject/brc-analytics
Auto-save: drop redundant Redis re-read, User load, history copies and refreshes per turn
- Dominant language
- TypeScript
- Stars
- 7
- Forks
- 11
- Avg merge
- 3d 15h
- Merged PRs (30d)
- 15
Description
Part of #1707. Found in the review of #1689.
## Problem
The signed-in auto-save path in `backend/api/app/api/v1/assistant.py` (around line 176) does redundant work on every turn:
- **Third session read.** It re-reads the session from Redis via `get_session` and runs `SessionState.model_validate` over the whole history, milliseconds after `_run_turn` wrote it (`assistant_agent.py:1194`).
- **Full `User` load.** `analysis_store.py:114` loads a whole `User` row where `get_user_id_by_keycloak_sub` exists and `turn_log.py:96` already uses it.
- **Double deep copy of the history.** `analysis_store.py:123–124` runs `to_jsonable_python` and then `strip_nuls` over `agent_message_history`, which is already `List[Dict]` per `models/assistant.py:233`. That's two full traversals of the largest per-session blob.
- **Pointless refresh.** `crud.py:261` / `crud.py:274` `session.refresh()` the full row after commit, re-selecting three JSON columns, even though `expire_on_commit=False` and the only consumer reads `.id`. `crud.py:360–362` already explains why this is unnecessary.
Related cleanup: `_get_saved_analysis_by_id` (`crud.py:183`) duplicates `get_saved_analysis`, and its post-rollback justification applies to no reachable path.
## Suggested fix
Hand the state out of `_run_turn` via an `on_state` callback like `on_turn`, pass `user_id` instead of loading `User`, drop the `to_jsonable_python` on the history, drop both refreshes, and fold `_get_saved_analysis_by_id` into `get_saved_analysis`.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with the signed-in auto-save path in backend/api/app/api/v1/assistant.py and _run_turn in assistant_agent.py, then inspect analysis_store.py, turn_log.py, and crud.py at the referenced lines. Trace how state, user_id, history, refreshes, and saved-analysis lookup flow through these paths. Done means the redundant Redis read, User load, history traversal, refreshes, and duplicate lookup are removed as described.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, redis
- Domain
- api, backend, database
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100